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

AdminUI - POC option groups/values page #25387

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

colemanw
Copy link
Member

Overview

POC replacement for option group administration pages, for discussion (because we haven't had enough of that on this subject yet!)

Before

Heavily overloaded path civicrm/admin/options is used to:

  1. Browse all option groups
  2. Browse the options in a group
  3. Create a new option group
  4. Edit an option group
  5. Create a new option in a group
  6. Edit an option
  7. Delete an option

After

(will update description as this changes)

Currently, this PR adds a new path: civicrm/admin/option (for POC purposes it's subtly different than the one in core) which is used to:

  1. Browse all option groups
  2. Browse the options in a group

I thought it could become a drop-in replacement for the currently overloaded path simply by adding the letter s when we're ready to flip that switch (provided we change items 3-7 to go through different paths). Now I'm wondering if we oughtn't just create an entirely new url structure because the old one really is bad.

How about this new url structure (similar to that of custom groups/fields):

  1. Browse all option groups: civicrm/admin/option/group
  2. Browse the options in a group civicrm/admin/option/group/values
  3. Create a new option group: civicrm/admin/option/group/add
  4. Edit an option group: civicrm/admin/option/group/update
  5. Create a new option in a group: civicrm/admin/option/group/values/add
  6. Edit an option: civicrm/admin/option/group/values/update
  7. Delete an option: civicrm/admin/option/group/values/delete

Ping @aydun

@civibot
Copy link

civibot bot commented Jan 19, 2023

(Standard links)

@eileenmcnaughton
Copy link
Contributor

I'm def +1 on the new urls - rather than replace

@eileenmcnaughton
Copy link
Contributor

Note I did have a PR to improve metadata sort of to support this - I just closed it as it keeps going stale & now tests are failing but it is here - I did already merge some bits

#24953

I hit an issue where read_only fields cannot be used as filters for form builder search views (what do we call those?). Also I think it almost needs a call back for read_only as some like contribution_status are read only at the NAME and VALUE level whereas others are not

@aydun
Copy link
Contributor

aydun commented Jan 20, 2023

Looks good! That gives us a framework for adapting the displays based on the values for the group (and without smarty!)

Changing the URL's makes it a larger effort but will give us more flexibility and allow us to override the path for the odd few that have their own dedicated edit pages. We will need to edit menus to show the new URL's. Any extensions that link to option edit pages will also need updating which could be tricky - they will need to point to different URL's depending on the version of core and whether or not AdminUI is enabled. But I doubt there are many like that.

I see the icon field in the tables now has a 'Option Value - Icon' option. Maybe we could do the same for 'Option Value - Colour' in the style box?

And this neatly solves the problem of getting the group description into the option editing display.

@eileenmcnaughton
Copy link
Contributor

@colemanw pleae merge this if you are happy with it based on @aydun's input. As this is wholly contained in an extension we are not yet enabling by default we have a little more leeway

@totten
Copy link
Member

totten commented Feb 21, 2023

I haven't r-run, but some feedback on this point:

Changing the URL's makes it a larger effort but will give us more flexibility and allow us to override the path for the odd few that have their own dedicated edit pages. We will need to edit menus to show the new URL's. Any extensions that link to option edit pages will also need updating which could be tricky - they will need to point to different URL's depending on the version of core and whether or not AdminUI is enabled. But I doubt there are many like that.

FWIW, when the CiviMail UI moved to JS (two million minutes ago), there was a similar issue. The approach there was to put in a server-side redirect for older entry-points that were hard to capture. The upshot of this approach: custom-menus/custom-pageflows still work. For AdminUI, it would have an extra benefit -- you can keep the same menus in place when folks enable/disable the AdminUI (because they're experimenting - and not sure if they're going to use it for long). That pattern could be stable-enough -- at least for the interim, until AdminUI becomes a standard.

@aydun
Copy link
Contributor

aydun commented Jun 5, 2023

Test this please

@aydun
Copy link
Contributor

aydun commented Jun 5, 2023

@colemanw There are a few comments from Eileen & Tim. Do you want to tweak further now, or merge this and follow up as needed?

@colemanw
Copy link
Member Author

colemanw commented Jun 7, 2023

@aydun I still feel like some tweaking of the paths in core is needed. I'll try to get to that soon...

@colemanw
Copy link
Member Author

Something related to this was just merged to master as b6e364d. That commit only works for custom fields though, so is more limited in scope.

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.

4 participants