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

[REF] Separate export form classes out & simplify task handling #18589

Merged
merged 2 commits into from
Oct 12, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 25, 2020

Overview

This separates out the rest of the export classes to be separate classes extending the parent. 2 functions are switched to overrride

Before

Lots of wrangling to get entity specific information. Task title involves lots of noise to do one boolean check

Screen Shot 2020-09-25 at 2 04 10 PM

After

Entity specific information is on the class (partially - in progress)

Technical Details

Comments

@colemanw

@civibot
Copy link

civibot bot commented Sep 25, 2020

(Standard links)

@civibot civibot bot added the master label Sep 25, 2020
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Sep 25, 2020
This variable is only used at the tpl layer in export - there is a separate PR
to change that usage to something more meaningful
civicrm#18589

Other than that all this task assignment appears to be just cruft

It's likely that it precedes other ways of setting the page title
@JoeMurray
Copy link
Contributor

@monishdeb could you review this? Thx

It turns out all this taskName assigning is just to compare it with 'Export Contacts' in the tpl

This makes it more direct
@eileenmcnaughton
Copy link
Contributor Author

@monishdeb did you get a chance to look at this?

@monishdeb
Copy link
Member

Jenkin test this please

@monishdeb
Copy link
Member

Sorry for delay. Tested export action with 'select fields' option on all desired entities like contact, activity, event, contribution, grant etc. without any issue. Merge action is rightfully disabled on the appropriate export entity page like activity, contribution etc. Overall, good initiative towards making entity specific export classes, towards adding more clarity. I am happy with the PR, merging now.

@monishdeb monishdeb merged commit 591bf4c into civicrm:master Oct 12, 2020
@eileenmcnaughton eileenmcnaughton deleted the ex_class branch October 12, 2020 20:56
@eileenmcnaughton
Copy link
Contributor Author

thanks @monishdeb

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.

3 participants