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

Hide deprecated fields from SearchKit & Afform #25113

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Dec 5, 2022

Overview

Hides any fields marked @deprecated from the SK & Afform UI. Follow-up to #25112.

Before

Now you see them.

After

Now you don't.

Technical Details

This builds on #24248 and #25112 and more recently #25112 by exposing the 'deprecated' attribute to APIv4 getfields and then filtering them out in SK & Afform.

@civibot
Copy link

civibot bot commented Dec 5, 2022

(Standard links)

@seamuslee001
Copy link
Contributor

@colemanw just hypothetically if someone had created a saved search kit using one of these fields would that mean the search display / Search kit breaks doing this because the field no longer renders in the Admin UI / any UI?

@colemanw
Copy link
Member Author

colemanw commented Dec 5, 2022

@seamuslee001 yes, the search will still run, but that field will disappear next time you edit it.

@demeritcowboy
Copy link
Contributor

  • This works
  • I have mixed feelings about is_current_revision. Not in terms of deprecating it, but in terms of it suddenly disappearing without warning if they have a search using it and edit it, which is more like removal/disabling rather than deprecation. I'd feel more comfortable with that one out for the moment.
  • Not a blocker but api4 explorer could benefit from some words or visual clue that the field is deprecated.

@colemanw
Copy link
Member Author

colemanw commented Dec 5, 2022

I have mixed feelings about is_current_revision. Not in terms of deprecating it, but in terms of it suddenly disappearing without warning if they have a search using it and edit it, which is more like removal/disabling rather than deprecation. I'd feel more comfortable with that one out for the moment.

OTOH, the longer we leave it in, the more likely someone is to use it. Do you know of anyone already doing so? I was assuming probably not.

@demeritcowboy
Copy link
Contributor

I see your point, and I forgot we already did

$preUpgradeMessage .= '<p>' . ts('The setting that used to be at <em>Administer &gt; CiviCase &gt; CiviCase Settings</em> for <strong>Enable deprecated Embedded Activity Revisions</strong> is enabled, but is no longer functional.<ul><li>For more information see this <a %1>Lab Snippet</a>.</li></ul>', [1 => 'target="_blank" href="https://lab.civicrm.org/-/snippets/85"']) . '</p>';
which will have caught some of that. It's just what's different about is_current_revision vs phone_id is that nobody who was just using core code will have anything in phone_id, whereas there will be people who have old revisions, with no way to exclude them from reporting except to do one of the things in the lab snippet mentioned by the earlier upgrade message.

So it doesn't block this PR but I'll look at doing an upgrade message for anyone who has an activity with is_current_revision=0, and point them to the snippet. The previous one was just for people with the setting still turned on.

@demeritcowboy demeritcowboy merged commit 19660c1 into civicrm:master Dec 6, 2022
@colemanw colemanw deleted the hideDeprecatedFields branch December 6, 2022 19:15
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