Skip to content

Fix answer choices default order#13955

Merged
alecslupu merged 2 commits intodevelopfrom
fix/answer_choices_default_order
Jan 29, 2025
Merged

Fix answer choices default order#13955
alecslupu merged 2 commits intodevelopfrom
fix/answer_choices_default_order

Conversation

@entantoencuanto
Copy link
Copy Markdown
Contributor

🎩 What? Why?

When application serializes answers to questions of sorting or matrix types there is not a default order defined and the database may return records with ids in reverse order causing misinterpretation of the results. This PR:

  • Defines a default scope in Decidim::Forms::AnswerChoice model, in ascending order first by position, then by position of matrix_row if any (used on matrix_single and matrix_multiple questions) and finally by id.
  • Adds some tests forcing positions not correlated with ids of records

Testing

Usually the database returns answer choices ordered by id and then errors do not occur. Try this with a database that does the opposite

📷 Screenshots

Screenshot from 2025-01-27 21-16-40

♥️ Thank you!
Screenshot from 2025-01-27 21-17-08

@entantoencuanto entantoencuanto added module: forms type: fix PRs that implement a fix for a bug labels Jan 27, 2025
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu left a comment

Choose a reason for hiding this comment

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

LGTM

@alecslupu alecslupu merged commit 570dd9e into develop Jan 29, 2025
@alecslupu alecslupu deleted the fix/answer_choices_default_order branch January 29, 2025 15:40
@alecslupu alecslupu added the release: v0.29 Issues or PRs that need to be tackled for v0.29 label Jan 29, 2025
@alecslupu alecslupu added the release: v0.28 Issues or PRs that need to be tackled for v0.28 label Jan 29, 2025
alecslupu added a commit that referenced this pull request Jan 30, 2025
entantoencuanto added a commit that referenced this pull request Jan 30, 2025
* feature/ephemeral_to_regular_users:
  Add integration test of ephemeral user authorization transfer
  Update test
  Make transferrable authorization when duplicate user is ephemeral
  Revert "Fix answer choices default order" (#13975)
  Fix DownloadYourData core pipeline failures (#13970)
  Update DownloadYourData exports for `decidim-proposals` (#13958)
  Update DownloadYourData exports for `decidim-initiatives` (#13961)
  Fix answer choices default order (#13955)
  Fix 'Conference media link creation form has wrong title' (#13946)
  Add etiquette validator to Debates and Meetings (#13274)
  Enable users to edit their survey answers (#13800)
  Fix "Missing padding-top on proposals page" (#13945)
antopalidi pushed a commit to openpoke/decidim that referenced this pull request Feb 12, 2025
andreslucena pushed a commit that referenced this pull request Mar 3, 2025
@alecslupu alecslupu added this to the 0.30.0 milestone Apr 28, 2025
@andreslucena
Copy link
Copy Markdown
Member

Adding the no-backport label as this is already in v0.30 and it was already backported to v0.28 and v0.29

@alecslupu alecslupu added the no-backport Pull Requests that should not be backported label Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: forms no-backport Pull Requests that should not be backported release: v0.28 Issues or PRs that need to be tackled for v0.28 release: v0.29 Issues or PRs that need to be tackled for v0.29 type: fix PRs that implement a fix for a bug

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants