Bug 36751 - Creating new patron attribute types should start with type choice
Summary: Creating new patron attribute types should start with type choice
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Pedro Amorim
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-02 11:04 UTC by Pedro Amorim
Modified: 2024-05-09 13:55 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 36751: PoC (9.07 KB, patch)
2024-05-02 11:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
UI demo screenshot (86.74 KB, image/png)
2024-05-02 11:10 UTC, Pedro Amorim
Details
Bug 36751: PoC (9.06 KB, patch)
2024-05-09 13:54 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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