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 Schema calculation of usePrefix to cope with rpow: #20471

Merged
merged 1 commit into from
Jun 18, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jun 2, 2021

Overview

This fixes the code in the logging schema to calculate $usePrefix based on whether the logging database name matches the civicrm database name, not the whole string.

$usePrefix is used to determine whether to prepend the database name in the triggers - ie

Before

database name is prepended into the string when rpow is in play

CREATE TRIGGER civicrm_activity_after_update ... THEN INSERT INTO `civi_database`.log_civicrm_activity 

This would also happen if different db users were used for the 2 databases (I can't think of any reason why they would be) - but the user difference would not flow through in any way.

After

CREATE TRIGGER civicrm_activity_after_update ... THEN INSERT INTO log_civicrm_activity 

Technical Details

Getting usePrefix right helps with portability as we avoid hard-coding the database name unless it is necessary.

The previous code was comparing the whole DSN - which will not match when (rpow)[https://github.com/totten/rpow] is in use. However, we are only using it to determine if the db name is the same (we don't write user or db host into the trigger anyway) & comparing just database gives a better result

Comments

I don't think a reviewer needs to test this in the rpow context - I think that as long as a reviewer experiences this as no change it's fine

@civibot
Copy link

civibot bot commented Jun 2, 2021

(Standard links)

@civibot civibot bot added the master label Jun 2, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 8, 2021
At some point these stopped being consistently alpha sorted - which doesn't matter
if you are just letting Civi run the trigger updates but if you output it
and diff it this inconsistency is a problem

Subset of civicrm#20472
in the hope of getting this merged

civicrm#20471 also grooms this output for diffing
albeit only in an edge case
@demeritcowboy
Copy link
Contributor

jenkins retest this please

@demeritcowboy demeritcowboy merged commit 9dd6542 into civicrm:master Jun 18, 2021
@eileenmcnaughton eileenmcnaughton deleted the schema branch June 19, 2021 23:35
@eileenmcnaughton
Copy link
Contributor Author

thanks @demeritcowboy !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants