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

Keep proposal new values for title and body when editing and receiving an error message #4592

Merged
merged 3 commits into from
Nov 30, 2018

Conversation

leio10
Copy link
Contributor

@leio10 leio10 commented Nov 29, 2018

🎩 What? Why?

When a user is trying to edit a proposal and receives an error message, values for title and body fields are reset to proposal original values, losing his last modifications.

This PR fixes this bug using the form value instead of the model value. This is done allowing the present helper to receive the form and the presenter class to be used.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add tests

📷 Screenshots (optional)

@ghost ghost assigned leio10 Nov 29, 2018
@ghost ghost added the status: WIP label Nov 29, 2018
mrcasals
mrcasals previously approved these changes Nov 29, 2018
Copy link
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this! ❤️

mrcasals
mrcasals previously approved these changes Nov 30, 2018
@leio10
Copy link
Contributor Author

leio10 commented Nov 30, 2018

@mrcasals it seems that the failed test is a flaky, can you please rerun it? Thanks!

@mrcasals
Copy link
Contributor

@leio10 re-scheduled!

@leio10
Copy link
Contributor Author

leio10 commented Nov 30, 2018

@mrcasals it failed again 😞, but I have no idea about that error. It doesn't seems to be related to this PR. What can I do to fix that? 🤔

@mrcasals
Copy link
Contributor

mrcasals commented Nov 30, 2018

Mhhh the problem seems to appear after merging #4578, let me ask @oriolgual and see if we find something.

The merge commit for that PR failed with the same exact error as this PR, but later merges are green 😕

@mrcasals
Copy link
Contributor

@leio10 #4600 should fix it!

@mrcasals
Copy link
Contributor

@leio10 PR #4600 merged, sorry about that! Can you rebase your PR please?

@leio10
Copy link
Contributor Author

leio10 commented Nov 30, 2018

Yeah! Now is green, thanks @mrcasals!!

@mrcasals mrcasals merged commit 4343a45 into decidim:master Nov 30, 2018
@ghost ghost removed the status: WIP label Nov 30, 2018
@mrcasals
Copy link
Contributor

Yay, merged! Thanks!!

@leio10 leio10 deleted the fix/edit_proposals branch November 30, 2018 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants