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

fix(client): Prevent saving invalid portfolio item #50110

Conversation

igorctorres
Copy link
Contributor

Checklist:

Unsaved portfolio item is being saved when updating an existing one, bypassing any validation.
This commit removes the unsaved item from portfolio before saving it, while the unsaved item is kept in state so user can keep editing it.

This bug was referenced on this comment: #49132 (comment)

Unsaved portfolio item is being saved when updating an existing one, bypassing
any validation.
This commit removes the unsaved item from portfolio before saving it, while the
unsaved item is kept in state so user can keep editing it.
@igorctorres igorctorres requested a review from a team as a code owner April 20, 2023 11:56
@github-actions github-actions bot added the platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. label Apr 20, 2023
@codesee-maps
Copy link
Contributor

codesee-maps bot commented Apr 20, 2023

👀 Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@moT01 moT01 added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Apr 20, 2023
@Sboonny
Copy link
Member

Sboonny commented Apr 20, 2023

Thank you for taking care of this, although in the future we can take care of this by creating two endpoints in the new API, your fix is welcomed 👍, and is helpful. thank you.


One nitpick, campers can update the existing items, when they create a new card in the profile page.

rec-tab.1.webm

@Sboonny Sboonny added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP and removed status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. labels Apr 20, 2023
@igorctorres
Copy link
Contributor Author

One nitpick, campers can update the existing items, when they create a new card in the profile page.

That's the current behavior and I don't see it as a problem.
When you update existing items, the new item is not saved along. The new item is temporarily removed from portfolio while the existing items are being saved. Then it is added back to the portfolio as a new item.

@Sembauke
Copy link
Member

@igorctorres Sorry for the lack of response. So as you can see from Sboonny's video, there seems to be a bug which adds the values of the previous card to the next one.

Though, the behaviour that is desired is to disable the ability to make new cards before saving the currently edited one. This should erase the above behaviour.

If you are still interested in developing this feature, then, go right ahead. If you are not, you can expect us to help. Please let us know.

Thank you, happy coding.

@Sembauke
Copy link
Member

Sembauke commented Jul 6, 2023

Hey @igorctorres

Thank you for taking the time to open this pull-request. Due to inactivity we will open this to other contributors. Feel free to revisit.

@Sembauke Sembauke closed this Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants