Export fix

[eileenmcnaughton]

Overview

Alternative to https://github.com/civicrm/civicrm-core/pull/17590/files

@colemanw the code that was hurting you actually only existed to support this class - so I ripped it out & It seems to work...

Before

After

Technical Details

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

This is incorrect; those two variables are not always the same, see for example CRM_Export_Controller_Standalone::$components['Membership'] == 'Member'

Granted the names of these variables is confusing. By "short name" they actually mean "component name"

[eileenmcnaughton]

@colemanw can you push up a fix to address that? same pattern I think (FWIW I think the whole switch could be in those functions....

[colemanw]

@eileenmcnaughton stepping through this code, every condition in this switch statement appears to be unreachable except for the default. I put a breakpoint in each of the switch conditions and couldn't trigger any of them, using standalone export or the various component search screens.

Are you seeing the same thing? Can we just remove the whole switch() and always use the default condition?

[colemanw]

@eileenmcnaughton I just pushed up a commit removing the switch entirely, as it appears to be unreachable.

[eileenmcnaughton]

I think you need to move this static to this form of 'somewhere' central if using from this form

[colemanw]

@eileenmcnaughton done

[eileenmcnaughton]

@colemanw that cleanup all looks good - but I'm not sure if @mattwire is leveraging the old logic (specifically mode_Query) from an extension. I know that was his motive in the first round of cleanup he did on this

[mattwire]

This needs careful review and testing. I'm not specifically using this code but was responsible for a lot of the earlier cleanup around this (starting with #11536). That switch statement is reachable from core (or at least was until very recently). - I'm sorry but I don't remember how... but likely through a combination of search tasks and entities.

I am very much in favour of trying to clean it up and/or remove it but we do need to steop carefully.

[mattwire]

Actually it's easy to reach - just use advanced search and select "display results as"->"contributions (or any other choice of entity) - that will set queryMode and take you to a specific case in that switch.

[colemanw]

Thanks @mattwire I've restored the switch so my changes are now fairly minimal.

[eileenmcnaughton]

Lets merge this then - I do think the switch could be improved but actually queryMode is more reliable than formName when I think about it. Anyway that has now been punted

[mattwire]

@eileenmcnaughton Agree I'd be happy to see this cleaned up further, but I remember it being somewhat non-trivial to debug!