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

[dev/core#457] Add $ids to hook_civicrm_export #12957

Merged
merged 1 commit into from
Oct 25, 2018

Conversation

scardinius
Copy link
Contributor

@scardinius scardinius commented Oct 18, 2018

Overview

Because of GDPR rules we have to save information that some data were exported.

Before

Current hook https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_export/ is not enough because it returns data without id columns. It's hard to implement any sophisticated feature without id of objects.

After

Existing export hook is extended

CRM_Utils_Hook::export($exportTempTable, $headerRows, $sqlColumns, $exportMode, $componentTable, $ids);

Technical Details

You can find example here veda-consulting-company/uk.co.vedaconsulting.gdpr#157

Comments

no comments

@civibot
Copy link

civibot bot commented Oct 18, 2018

(Standard links)

@civibot civibot bot added the master label Oct 18, 2018
@pradpnayak
Copy link
Contributor

@scardinius Doesn't the $exportTempTable table have id of the component?

@scardinius
Copy link
Contributor Author

@pradpnayak it depends on type of exporting object. As I remember only temporary table for contacts contains id

@pradpnayak
Copy link
Contributor

Probably with different column name? eg activity_id, contribution_id.

@scardinius I think we should extend export code to store id for component in $exportTempTable table so that it would be useful for other need if some wants to play around with it like not to export certain rows using component id etc using hook_civicrm_export.

Thoughts?

@colemanw
Copy link
Member

I tend to agree that the existing hook should be improved instead of adding a redundant one.

@scardinius
Copy link
Contributor Author

Hi, thanks for your suggestions. I've just move these two new variables into export hook

@colemanw
Copy link
Member

Makes sense to me 👍

@colemanw
Copy link
Member

@scardinius it would be helpful if you could squash the commits from this PR into one so we can have a nice commit history in master.

@colemanw colemanw changed the title [dev/core#457] Add hook exportIds needed for GDPR [dev/core#457] Add $ids to hook_civicrm_export Oct 22, 2018
@scardinius
Copy link
Contributor Author

@colemanw you're right, I'll squash them

@scardinius
Copy link
Contributor Author

@colemanw I've just squashed the commits

@colemanw
Copy link
Member

Verified that the variables are always defined and that this function is only called in that one place, so this is safe to merge.

@colemanw colemanw merged commit f742ab0 into civicrm:master Oct 25, 2018
@colemanw
Copy link
Member

@scardinius
Copy link
Contributor Author

@colemanw sure, I will

@scardinius
Copy link
Contributor Author

@colemanw doc updated civicrm/civicrm-dev-docs#560

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