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

CustomFields - Drop unused column 'mask' #25396

Merged
merged 1 commit into from Jan 25, 2023

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jan 20, 2023

Overview

Drops a column which doesn't appear to be used for anything.

Before

mask is added to the CRM/Custom/Form/Field.php quickform but never printed in the corresponding CRM/Custom/Form/Field.tpl, and does not appear to be used anywhere else. I don't see any reference to it in CRM_Core_BAO_CustomField when the custom fields actually get rendered.

After

Buh bye.

This column doesn't appear to be used for anything.
@civibot
Copy link

civibot bot commented Jan 20, 2023

(Standard links)

@civibot civibot bot added the master label Jan 20, 2023
@yashodha
Copy link
Contributor

@colemanw Is this something some extensions could have used may be?

@colemanw
Copy link
Member Author

@yashodha I searched through universe and couldn't find it used anywhere.

@yashodha
Copy link
Contributor

@colemanw merging this PR.

@yashodha yashodha merged commit 7439108 into civicrm:master Jan 25, 2023
@colemanw colemanw deleted the customMaskDrop branch January 25, 2023 13:12
@MegaphoneJon
Copy link
Contributor

This breaks upgrades when advanced logging is enabled (and I would guess possibly multilingual). I think the dropColumn function needs to immediately update the logging schema and rebuild triggers. I'll open a Gitlab ticket.

@seamuslee001
Copy link
Contributor

hmm @MegaphoneJon so similar to this patch #25421 but slightly different context but same problem right?

@MegaphoneJon
Copy link
Contributor

@seamuslee001 Yes, that's correct. I think maybe alterColumn and dropColumn should have those lines themselves rather than relying on their being added to the upgrade step.

@colemanw
Copy link
Member Author

colemanw commented Feb 7, 2023

I'd be ok with adding the logging/schema/rebuild functionality to dropColumn and maybe alterColumn. I agree it would benefit long-term stability but might cause some short-term upset if we don't thoroughly check all the places those functions are used and verify the change is ok.

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