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#1549 - Malleate civicrm_generated so that long lines are split #20513

Merged
merged 1 commit into from
Jun 6, 2021

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/1549

Long lines are a problem for files in version control since it's hard to see what changed and can quickly lead to conflicts between competing PRs.

Using mysqldump separate full inserts for each row is much slower - this still uses extended inserts just adds newlines for each row.

There was some work to eliminate civicrm_generated.mysql completely, but it's not there yet.

Before

INSERT INTO `civicrm_acl` (`id`, `name`, `deny`, `entity_table`, `entity_id`, `operation`, `object_table`, `object_id`, `acl_table`, `acl_id`, `is_active`) VALUES (1,'Edit All Contacts',0,'civicrm_acl_role',1,'Edit','civicrm_saved_search',0,NULL,NULL,1),(2,'Core ACL',0,'civicrm_acl_role',0,'All','access CiviMail subscribe/unsubscribe pages',NULL,NULL,NULL,1),(3,'Core ACL',0,'civicrm_acl_role',0,'All','access all custom data',NULL,NULL,NULL,1),(4,'Core ACL',0,'civicrm_acl_role',0,'All','make online contributions',NULL,NULL,NULL,1),(5,'Core ACL',0,'civicrm_acl_role',0,'All','make online pledges',NULL,NULL,NULL,1),(6,'Core ACL',0,'civicrm_acl_role',0,'All','profile listings and forms',NULL,NULL,NULL,1),(7,'Core ACL',0,'civicrm_acl_role',0,'All','view event info',NULL,NULL,NULL,1),(8,'Core ACL',0,'civicrm_acl_role',0,'All','register for events',NULL,NULL,NULL,1),(9,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviCRM',NULL,NULL,NULL,1),(10,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviContribute',NULL,NULL,NULL,1),(11,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviEvent',NULL,NULL,NULL,1),(12,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviMail',NULL,NULL,NULL,1),(13,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviMail subscribe/unsubscribe pages',NULL,NULL,NULL,1),(14,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviMember',NULL,NULL,NULL,1),(15,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviPledge',NULL,NULL,NULL,1),(16,'Core ACL',0,'civicrm_acl_role',1,'All','administer CiviCase',NULL,NULL,NULL,1),(17,'Core ACL',0,'civicrm_acl_role',1,'All','access my cases and activities',NULL,NULL,NULL,1),(18,'Core ACL',0,'civicrm_acl_role',1,'All','access all cases and activities',NULL,NULL,NULL,1),(19,'Core ACL',0,'civicrm_acl_role',1,'All','delete in CiviCase',NULL,NULL,NULL,1),(20,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviGrant',NULL,NULL,NULL,1),(21,'Core ACL',0,'civicrm_acl_role',1,'All','access Contact Dashboard',NULL,NULL,NULL,1),(22,'Core ACL',0,'civicrm_acl_role',1,'All','administer Multiple Organizations',NULL,NULL,NULL,1),(23,'Core ACL',0,'civicrm_acl_role',1,'All','delete activities',NULL,NULL,NULL,1),(24,'Core ACL',0,'civicrm_acl_role',1,'All','delete in CiviContribute',NULL,NULL,NULL,1),(25,'Core ACL',0,'civicrm_acl_role',1,'All','delete in CiviMail',NULL,NULL,NULL,1),(26,'Core ACL',0,'civicrm_acl_role',1,'All','delete in CiviPledge',NULL,NULL,NULL,1),(27,'Core ACL',0,'civicrm_acl_role',1,'All','delete contacts',NULL,NULL,NULL,1),(28,'Core ACL',0,'civicrm_acl_role',1,'All','delete in CiviEvent',NULL,NULL,NULL,1),(29,'Core ACL',0,'civicrm_acl_role',1,'All','delete in CiviMember',NULL,NULL,NULL,1),(30,'Core ACL',0,'civicrm_acl_role',1,'All','translate CiviCRM',NULL,NULL,NULL,1),(31,'Core ACL',0,'civicrm_acl_role',1,'All','edit grants',NULL,NULL,NULL,1),(32,'Core ACL',0,'civicrm_acl_role',1,'All','access all custom data',NULL,NULL,NULL,1),(33,'Core ACL',0,'civicrm_acl_role',1,'All','access uploaded files',NULL,NULL,NULL,1),(34,'Core ACL',0,'civicrm_acl_role',1,'All','add contacts',NULL,NULL,NULL,1), ... etc ...

and that goes on for a while, then repeat for each table.

After

INSERT INTO `civicrm_acl` (`id`, `name`, `deny`, `entity_table`, `entity_id`, `operation`, `object_table`, `object_id`, `acl_table`, `acl_id`, `is_active`) VALUES
 (1,'Edit All Contacts',0,'civicrm_acl_role',1,'Edit','civicrm_saved_search',0,NULL,NULL,1),
 (2,'Core ACL',0,'civicrm_acl_role',0,'All','access CiviMail subscribe/unsubscribe pages',NULL,NULL,NULL,1),
 (3,'Core ACL',0,'civicrm_acl_role',0,'All','access all custom data',NULL,NULL,NULL,1),
 (4,'Core ACL',0,'civicrm_acl_role',0,'All','make online contributions',NULL,NULL,NULL,1),
 (5,'Core ACL',0,'civicrm_acl_role',0,'All','make online pledges',NULL,NULL,NULL,1),
 (6,'Core ACL',0,'civicrm_acl_role',0,'All','profile listings and forms',NULL,NULL,NULL,1),
 (7,'Core ACL',0,'civicrm_acl_role',0,'All','view event info',NULL,NULL,NULL,1),
 (8,'Core ACL',0,'civicrm_acl_role',0,'All','register for events',NULL,NULL,NULL,1),
 (9,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviCRM',NULL,NULL,NULL,1),
 (10,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviContribute',NULL,NULL,NULL,1),
 (11,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviEvent',NULL,NULL,NULL,1),
 (12,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviMail',NULL,NULL,NULL,1),
 (13,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviMail subscribe/unsubscribe pages',NULL,NULL,NULL,1),
 (14,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviMember',NULL,NULL,NULL,1),
 (15,'Core ACL',0,'civicrm_acl_role',1,'All','access CiviPledge',NULL,NULL,NULL,1),
 (16,'Core ACL',0,'civicrm_acl_role',1,'All','administer CiviCase',NULL,NULL,NULL,1),
 (17,'Core ACL',0,'civicrm_acl_role',1,'All','access my cases and activities',NULL,NULL,NULL,1),
 ...etc...

Technical Details

Definition of malleate: #20510 (comment)

Comments

There's maybe some risk but:

  • Does anyone install the sample data on a live site?
  • It's pretty well tested by tests since all the test runs use the sample data.

@civibot
Copy link

civibot bot commented Jun 5, 2021

(Standard links)

@civibot civibot bot added the master label Jun 5, 2021
@totten
Copy link
Member

totten commented Jun 6, 2021

@demeritcowboy Yeah, I think this will reduce some concerns around git conflicts. 👍

Worth noting that civicrm_generated.mysql actually includes the row IDs and random data. I imagine there will be a mix of cases - some with reduced conflict-iness (e.g. changing the label on an OptionValue) and some with large-looking conflicts (e.g. adding a new OptionValue in the middle of the list). But even in case of large-looking conflicts, it should still be more readable...

r-run: I logged into the autobuild demo and it still seemed to have the ordinary amount of sample data, so the basic syntax is still valid/working. ✅

I agree that the technique here is a bit risky, and I'd be 👎 for using on production workflows. But this filter only applies to a closed and synthetic test-data-set, and (as you point out) the whole thing is likely to be omitted in the future. Proper parsing would require considerably more code and is unlikely to give a tangible benefit... so let's go with it.

@totten totten merged commit 9425724 into civicrm:master Jun 6, 2021
@demeritcowboy demeritcowboy deleted the regen-better branch June 7, 2021 14:46
@demeritcowboy
Copy link
Contributor Author

Thanks. Yeah it's only a tiny improvement.

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