zubhub icon indicating copy to clipboard operation
zubhub copied to clipboard

Validate all activity creation input fields

Open srish opened this issue 1 year ago • 9 comments

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 avatar Oct 13 '23 22:10 srish

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?

brrkrmn avatar Oct 14 '23 01:10 brrkrmn

I think this is also handled in here https://github.com/unstructuredstudio/zubhub/pull/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 .

coderatomy avatar Oct 14 '23 17:10 coderatomy

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

yokwejuste avatar Oct 14 '23 19:10 yokwejuste

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

brrkrmn avatar Oct 17 '23 11:10 brrkrmn

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

definitely :)

yokwejuste avatar Oct 17 '23 12:10 yokwejuste

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

coderatomy avatar Oct 19 '23 15:10 coderatomy

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

rak16 avatar Oct 19 '23 18:10 rak16

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.

yokwejuste avatar Oct 19 '23 19:10 yokwejuste

roger that @rak16

coderatomy avatar Oct 19 '23 20:10 coderatomy