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

SavedSearch - Add name and label columns #18809

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Oct 20, 2020

Overview

Adds 2 database columns: name and label for the SavedSearch entity.
This is needed by the new Search Kit extension.

Before

SavedSearch entity lacks unique name or label.
Pre/post hooks not called for SavedSearch entity.

After

Label column added for identifying saved search on administrative screens.
Unique name column added for embedding savedsearch displays by name on pages, dashlets, etc.
Pre/post hooks called.

Technical Details

Unique name generated from label if not supplied.
Switching from ad-hoc create function to the standard dao::writeRecord ensures that hooks are properly called.

@civibot
Copy link

civibot bot commented Oct 20, 2020

(Standard links)

@civibot civibot bot added the master label Oct 20, 2020
@colemanw
Copy link
Member Author

@seamuslee001 @eileenmcnaughton @totten this is going to go stale real fast.

</index>

<field>
<name>label</name>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need "localizable"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I don't think so. This is intended for back-end administration screens.

@colemanw
Copy link
Member Author

Test fails were being caused by the sloppy function CRM_Contact_BAO_Group::createSmartGroup(). I cleaned it up to fix the failures, but it's currently only ever used by tests, so we could delete it and refactor the tests to use something else, or move it into the test class and out of the BAO.

@eileenmcnaughton eileenmcnaughton merged commit 0862a72 into civicrm:master Oct 20, 2020
@eileenmcnaughton eileenmcnaughton deleted the savedSearchName branch October 20, 2020 19:39
@colemanw
Copy link
Member Author

@eileenmcnaughton I forgot to add the unique index to the upgrader. Looking at the function CRM_Upgrade_Incremental_Base::addIndex() it doesn't appear capable of creating a UNIQUE index.
But I see there's also a function CRM_Core_BAO_SchemaHandler::createMissingIndices() which looks like it would automatically add the index based on the schema... but I don't see it being called from the upgrader? Is it ever called routinely, or only manually?

@eileenmcnaughton
Copy link
Contributor

@colemanw only manually

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