Skip to content

Conversation

@yoution
Copy link
Collaborator

@yoution yoution commented Jan 4, 2021

@maxceem please review for #4236

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoution great fix.

There is only one thing to improve. When I update project status using admin user (maybe other too), additional data which we load for some members got lost. See demo video https://monosnap.com/file/f8Z2U9EVbZasx8oTcY5wcBuZlX5i8p.

I think it happens because after loading project list, we additionally load some user details and put it inside project data in Redux store.

So we have to 2 way so go:

  • after updating project reload user data for it.
  • or when we update project we can get members from the previous project data, and apply it to the updated project data, so we don't have to reload members.

Maybe you have another idea.

@yoution
Copy link
Collaborator Author

yoution commented Jan 5, 2021

@maxceem please review again

@maxceem
Copy link
Collaborator

maxceem commented Jan 5, 2021

Thanks for the update, @yoution. Would it be hard to update the project object as it was done before and just keep members?

I like the previous way as it was more universal, if tomorrow we update something else from this page. Also, last updated time is updating during status update and the user who did the last update. So it would be nice if all these data got updated.

@yoution
Copy link
Collaborator Author

yoution commented Jan 5, 2021

@maxceem I know ,I will update soon

@yoution
Copy link
Collaborator Author

yoution commented Jan 5, 2021

@maxceem please review again again

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @yoution, works perfectly.

@maxceem maxceem merged commit e7ef51a into topcoder-archive:dev Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants