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 fatal error when sorting by status in activity search #15911

Closed
wants to merge 2 commits into from

Conversation

pfigel
Copy link
Contributor

@pfigel pfigel commented Nov 21, 2019

Overview

This fixes an issue where sorting activity search results by activity status fails with a fatal error because the pseudoconstants are not loaded with the correct field name.

Before

Sorting activity search results by activity status results in a fatal error.

After

Sorting by activity status works.

Technical Details

The underlying issue is that prepareOrderBy() hits buildOptions() with activity_status, but only activity_status_id or status_id would actually return the pseudoconstants. This builds on top of #15899 and sets where for the field as it would otherwise cause a DB error as well (since activity_status is not the name of the column).

Comments

Not entirely sure about this fix - perhaps there's a better place to apply these changes or set an alias for this column. The pseudoconstant bit of the fix could also be done by adding a special case for activity_status in CRM_Activity_BAO_Activity::buildOptions(), but that feels a bit hacky.

I put this against 5.20 since it's similar to #15899, but this regression has been around a bit longer (confirmed broken on 5.13), so I can rebase if needed.

This fixes an issue where columns whose names are not unique in a
search query cause a DB error when they're used as a sort column.
The issue can be observed in the contribution search when sorting
by contribution status.

The issue is resolved by using the where field of the column spec,
which holds the fully-qualified name of the column.
This fixes an issue where sorting activity search results by activity
status fails with a fatal error because the pseudoconstants are not
loaded with the correct field name.
@civibot
Copy link

civibot bot commented Nov 21, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@pfigel alternate proposed solution - #15923

@pfigel
Copy link
Contributor Author

pfigel commented Nov 22, 2019

Closing in favour of #15923

@pfigel pfigel closed this Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants