Bug 36751

Summary: Creating new patron attribute types should start with type choice
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: PatronsAssignee: Pedro Amorim <pedro.amorim>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, julian.maurice, kyle.m.hall, martin.renvoize, nick, oleonard
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36761
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 36751: PoC
UI demo screenshot
Bug 36751: PoC

Description Pedro Amorim 2024-05-02 11:04:24 UTC
A date attribute type cannot be text, an authorised value category attribute type cannot be date, etc.

As such, we should ideally not keep adding options to the attribute type form that can't make sense together.
Currently, an authorised value category attribute can be set as "repeatable" but it doesn't work.
A future use-case example: we add the option for a "True/False" attribute type, this type should not possibly be a date, an authorised value category, repeatable and should also not possibly be marked as a unique identifier.
Comment 1 Pedro Amorim 2024-05-02 11:09:30 UTC
Created attachment 166061 [details] [review]
Bug 36751: PoC

This is a PoC only. More work should be done here.
Imo, ideally, we'd have a mapping where each 'model' would define what it supports ('repeateable', 'searchable', 'unique identifier', etc).
But first would like to know what others think of this. Is this is a good idea? Should we invest more time here.

Test plan, k-t-d:
  Visit patron attribute types at:
http://localhost:8081/cgi-bin/koha/admin/patron-attr-types.pl
  Click "New patron attribute type"
  Notice you are now presented with choices in regards to the model of the attribute type: "Text", "Date" or "Authorized value category
  Notice if you pick "Date", you're unable to pick an authorized value category
  Notice if you pick "Authorised value category", you're unable to pick if its a date or not
Comment 2 Pedro Amorim 2024-05-02 11:10:18 UTC
Created attachment 166062 [details]
UI demo screenshot
Comment 3 Jonathan Druart 2024-05-07 09:12:52 UTC
Yes, good idea, but I would put everything on the same view. The buttons on top of the form maybe?
Comment 4 Pedro Amorim 2024-05-08 10:54:42 UTC
(In reply to Jonathan Druart from comment #3)
> Yes, good idea, but I would put everything on the same view. The buttons on
> top of the form maybe?

Interesting suggestion, my idea was to do exactly the opposite of this, i.e. show the user the least possible UI elements at a time, to 'ease in' creating a new custom field, and show only the UI applicable elements once the type/model choice was done.

If we show everything on the same view, we'd then need to hide unapplicable UI elements depending on the selected type through JavaScript (?) which is exactly what I'd like to avoid doing, e.g. if the user picks "Date", then the "Authorized value" input would be hidden by JS, and vice-versa.
This is what is happening currently, but disabling/enabling UI elements instead of hiding, through JS. Which, again, is exactly what I'm trying to improve here.
Comment 5 Nick Clemens (kidclamp) 2024-05-08 11:46:46 UTC
I agree with Pedro, the current form disabling some optoins as you enable others is unclear.

I also agree with Joubu - the full intermediate page seems a bit too far.

I think if you made the 'New patron attribute type' a split button - so you had a dropdown and could initially select the type before going to the form it would let you skip the extra page, but could also direct you there if you clicked the main button. Does that make sense?
Comment 6 Martin Renvoize (ashimema) 2024-05-09 11:07:55 UTC
I like the idea of the split button approach :)
Comment 7 Pedro Amorim 2024-05-09 13:54:45 UTC
Created attachment 166457 [details] [review]
Bug 36751: PoC

New PoC removing the intermediary screen and adding dropdown choices instead.
This also now adds the 'model' column in the UI table.

I would prefer to go deeper and update the database schema but I feel that might be too much for now.

Test plan, remains the same but dropdown choices instead of new UI screen:
  Visit patron attribute types at:
http://localhost:8081/cgi-bin/koha/admin/patron-attr-types.pl
  Click "New patron attribute type"
  Notice you are now presented with choices in regards to the model of the attribute type: "Text", "Date" or "Authorized value category
  Notice if you pick "Date", you're unable to pick an authorized value category
  Notice if you pick "Authorised value category", you're unable to pick if its a date or not