Skip to content

Fix proposals creation#6585

Merged
tramuntanal merged 2 commits intodevelopfrom
proposals/fix-creation-update
Oct 6, 2020
Merged

Fix proposals creation#6585
tramuntanal merged 2 commits intodevelopfrom
proposals/fix-creation-update

Conversation

@mrcasals
Copy link
Copy Markdown
Contributor

@mrcasals mrcasals commented Oct 2, 2020

🎩 What? Why?

Proposals get wrongly created from the public area. This PR fixes the problem.

📌 Related Issues

Testing

Create a proposal from the public area. See that in the DB its title and body are strings instead of being key-value hashes.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

None

@mrcasals mrcasals mentioned this pull request Oct 2, 2020
Merged
13 tasks
oriolgual
oriolgual previously approved these changes Oct 2, 2020
View reviewed changes
@mrcasals
Copy link
Copy Markdown
Contributor Author

mrcasals commented Oct 2, 2020

@decidim/core can you review this, please? It's a bugfix for proposal creation

@mrcasals
Copy link
Copy Markdown
Contributor Author

mrcasals commented Oct 2, 2020 •
edited
Loading

@decidim/core Meetings is failing again due to that spec...

@tramuntanal tramuntanal self-assigned this Oct 6, 2020
tramuntanal
tramuntanal previously approved these changes Oct 6, 2020
View reviewed changes
@mrcasals
Copy link
Copy Markdown
Contributor Author

mrcasals commented Oct 6, 2020

@tramuntanal did you reestart the meetings suite? It's probably going to fail until we merge #6595 and rebase our PRs... Can we merge this one, please?

@mrcasals mrcasals dismissed stale reviews from tramuntanal and oriolgual via e2e82ed October 6, 2020 08:25
@mrcasals mrcasals force-pushed the proposals/fix-creation-update branch from d53d0d3 to e2e82ed Compare October 6, 2020 08:25
@mrcasals
Copy link
Copy Markdown
Contributor Author

mrcasals commented Oct 6, 2020

@decidim/core I see you keep restarting the Meetings job. I've rebased the PR with develop so it gets fixed.

@mrcasals
Copy link
Copy Markdown
Contributor Author

mrcasals commented Oct 6, 2020

@decidim/core all tests are green now! Can you re-review this please? 😄

@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Oct 6, 2020

Thanks @mrcasals !

@tramuntanal tramuntanal merged commit dd356e6 into develop Oct 6, 2020
@tramuntanal tramuntanal deleted the proposals/fix-creation-update branch October 6, 2020 10:03
roxanaopr pushed a commit to i-need-another-coffee/decidim that referenced this pull request Oct 15, 2020
@mrcasals @roxanaopr
Fix proposals creation (decidim#6585)
614f084 
* Fix proposals creation

* Add migration
@mrcasals mrcasals added module: proposals type: fix PRs that implement a fix for a bug labels Feb 25, 2021
@ahukkanen
Copy link
Copy Markdown
Contributor

ahukkanen commented Mar 2, 2021

Not that it matters that much but I think this PR duplicated a migration from:
#6518

Now we have both:
https://github.com/decidim/decidim/blob/fee1047baea94e98774d9ebd256e399098718971/decidim-proposals/db/migrate/20200915151348_fix_proposals_data_to_ensure_title_and_body_are_hashes.rb
AND
https://github.com/decidim/decidim/blob/fee1047baea94e98774d9ebd256e399098718971/decidim-proposals/db/migrate/20201002085508_fix_proposals_data.rb

Which both seem the same to me.

@mrcasals
Copy link
Copy Markdown
Contributor Author

mrcasals commented Mar 3, 2021

@ahukkanen you're right, sorry. I'll send PRs removing one of them, so at least it will be fixed for any app that hasn't upgraded yet.

@mrcasals mrcasals mentioned this pull request Mar 3, 2021
Merged
@oliverbarnes oliverbarnes mentioned this pull request Mar 16, 2021
Closed
@ahukkanen ahukkanen mentioned this pull request May 14, 2021
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@tramuntanal tramuntanal tramuntanal approved these changes

@oriolgual oriolgual oriolgual left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@tramuntanal tramuntanal

Labels

in-review module: proposals type: fix PRs that implement a fix for a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mrcasals @tramuntanal @ahukkanen @oriolgual