Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate all activity creation input fields #898

Open
srish opened this issue Oct 13, 2023 · 9 comments
Open

Validate all activity creation input fields #898

srish opened this issue Oct 13, 2023 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers high priority

Comments

@srish
Copy link
Member

srish commented Oct 13, 2023

This task is for developers.

Currently, if any mandatory fields in the activity creation field are left empty (image, text, etc.), activity creation still goes through without showing errors, but the activity doesn't get created.

This task is about making sure that all the fields are properly validated. When you click on publish activity, you get an error in the API response showing what is missing. That error needs to be caught as well.

For design reference, see how errors are handled for project creation.

@srish srish added enhancement New feature or request good first issue Good for newcomers high priority labels Oct 13, 2023
@brrkrmn
Copy link
Collaborator

brrkrmn commented Oct 14, 2023

The only mandatory fields are title and description in my case. I can create activities with only these two fields. 🤔 Does anyone get a different result?

@coderatomy
Copy link
Collaborator

coderatomy commented Oct 14, 2023

I think this is also handled in here #885 @brrkrmn @srish . I wrapped most of the activity work
But as @brrkrmn said, I think we need clarity on what's required and not @srish .

@yokwejuste
Copy link
Collaborator

yokwejuste commented Oct 14, 2023

I think this is also handled in here #885 @brrkrmn @srish . I wrapped most of the activity work But as @brrkrmn said, I think we need clarity on what's required and not @srish .

so how do we go about this? @coderatomy @srish @brrkrmn

@brrkrmn
Copy link
Collaborator

brrkrmn commented Oct 17, 2023

@coderatomy, @yokwejuste, maybe we can wait for #885 to conclude and then move on with those changes, or get more instructions from Srish

@yokwejuste
Copy link
Collaborator

@coderatomy, @yokwejuste, maybe we can wait for #885 to conclude and then move on with those changes, or get more instructions from Srish

definitely :)

@coderatomy
Copy link
Collaborator

So @srish @yokwejuste @brrkrmn. I have finally finalised work here. I have made all the fields required since I had no clear way forward. Feel free to checkout the fix #885

@rak16
Copy link

rak16 commented Oct 19, 2023

Hey @coderatomy I don't think we should keep adding all the fixes related to activity in the same PR. I have noticed that you have included changes from other issues (like this one) too under the same PR.

I understand that you are trying to logically club the changes to address all of them at once but it has quite a few downsides. It makes it difficult for the reviewers, introduces regression issues and makes it difficult to roll back changes, amongst other things.

PR's should be as concise as possible. It's a good idea to follow single-responsibility principle not only in the code but in the PR's too. So let's keep it at one PR per issue as a rule of thumb.

@brrkrmn @aqsaaqeel @yokwejuste Something to keep in mind for you people too.

cc: @tuxology @kamthamc

@yokwejuste
Copy link
Collaborator

Hey @coderatomy I don't think we should keep adding all the fixes related to activity in the same PR. I have noticed that you have included changes from other issues (like this one) too under the same PR.

I understand that you are trying to logically club the changes to address all of them at once but it has quite a few downsides. It makes it difficult for the reviewers, introduces regression issues and makes it difficult to roll back changes, amongst other things.

PR's should be as concise as possible. It's a good idea to follow single-responsibility principle not only in the code but in the PR's too. So let's keep it at one PR per issue as a rule of thumb.

@brrkrmn @aqsaaqeel @yokwejuste Something to keep in mind for you people too.

cc: @tuxology @kamthamc

sure, got it.

I share that opinion too to make it as more focus as possible.

@coderatomy
Copy link
Collaborator

roger that @rak16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers high priority
Projects
None yet
Development

No branches or pull requests

5 participants