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

Add explicit order by id when as dependant fields have same field order #11437

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

rohitpavaskar
Copy link
Contributor

@rohitpavaskar rohitpavaskar commented Sep 6, 2022

Q A
Bug fix? (use the a.b branch) [Y]
New feature/enhancement? (use the a.x branch) [N]
Deprecations? [N]
BC breaks? (use the c.x branch) [N]
Automated tests included? [N]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

This PR fix issue with ordering of fields data for form results on form results page.
This issue is reproducible only for form which is not reproducible on another forms as it depends on how MySQL stores and reads data. When we have two fields with same field order which is in case of dependant fields e.g. Country and state the data fetched will have different order than ID as fallback ordering. So explicitly specifying fallback order will solve the issue.

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Create a form with Country field and add state as dependant field and verify if ordering is correct after form submission,

Note:
Test cases covering this code are already present so havn't added any test cases and it is not possible to create exact same scenario also.

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Sep 6, 2022
@codecov
Copy link

codecov bot commented Sep 6, 2022

Codecov Report

Merging #11437 (5d66b24) into 4.4 (ba7541f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                4.4   #11437   +/-   ##
=========================================
  Coverage     49.36%   49.36%           
  Complexity    35401    35401           
=========================================
  Files          2144     2144           
  Lines        105532   105532           
=========================================
  Hits          52101    52101           
  Misses        53431    53431           
Impacted Files Coverage Δ
...bundles/FormBundle/Entity/SubmissionRepository.php 56.91% <100.00%> (ø)

@escopecz escopecz added bug Issues or PR's relating to bugs forms Anything related to forms labels Sep 6, 2022
Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

Clear fix. Thanks Rohit!

@escopecz escopecz added this to the 4.4.3 milestone Sep 6, 2022
@escopecz escopecz merged commit ea3d85c into mautic:4.4 Sep 6, 2022
@escopecz escopecz added the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement forms Anything related to forms ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants