msupply-foundation / conforma-server

Conforma application manager (IRIMS) back-end
GNU Affero General Public License v3.0
4 stars 1 forks source link

Epic -- Editing lookup tables #629

Open CarlosNZ opened 2 years ago

CarlosNZ commented 2 years ago

This is something that seems to be wanted soon, so here's my suggestion of how it can work:

The easiest way to view lookup table contents is using Outcomes, since that's already built and can be made to display lookup table data with no extra development (just config). However editing is a bit trickier, but it makes sense to extend the outcome UI to handle this too.

So, with that in mind, here is my list (in rough order) of what I think needs to happen to make this work:

  1. When a new lookup table is imported for the first time, generate, an "Outcome_display" configuration automatically, so lookup tables can be immediately browsed with no additional config work required. (Time estimate: 0.5 day)
  2. Outcome table needs to be sortable, filter-able/searchable, since lookup tables could be hundreds or even thousands of items, and there's no point being able to edit if it's hard to find what you need. (Can copy a lot of the filter/sort functionality from Application List for this) (Time estimate: 2 days)
  3. Add a field to "outcome_display" table called "edit_permission_names", specifying what permissions are required to be allowed to edit outcome table data. Default for normal outcomes should be NONE (i.e. not editable at all), and for lookup_table displays can be "admin" (Format is same as existing "permission_names" field -- an array of permission names) (Time estimate: 1-2 hours )
  4. If a user is allowed to edit an Outcome display (based on above permission check), then when in "Outcome Detail" view (i.e. a single item), then each field can be made editable by clicking in it and changing the value. (Needs some thought with specially formatted cells, like SummaryView elements, but for lookup tables, it's mostly going to be plain text, so shouldn't be a big problem). When a cell is first changed, a "Save Changes" and a "Cancel" button become visible. Nothing is saved back to the database until user explicitly clicks "Save Changes". For safety, pressing "Cancel" gives a "you will lose unsaved changes" warning. (Time estimate: 2 days)
  5. Add "Add" and "Delete" records functionality. UI for delete is easy -- just a trash can icon in Detail view, with a confirmation before actually deleting. "Add" would bring up a "Details" view, but with empty fields which must be filled in before saving. (Time estimate: 1 day)

Question: How do we handle row-level policy generation for new lookup tables (and outcome tables more generally too)?

Thoughts? Anything I've missed? We can have a chat about how to proceed at next daily meetup if you like.

nmadruga commented 2 years ago

Looks great Carl, I really like the idea on point 1: to have an automatically configured "outcome_display" set created.

2 things to add:

CarlosNZ commented 2 years ago

How would the user do Addition/Removal of records in the Lookup table?

I guess also in Issue 4, we'd just add a "+" and "delete" icon as well. Perhaps that can be Issue 5. I'll add it.

Would this be different permissions

I was thinking that the allowed permissions would just cover "editing", which includes, altering, adding and deleting. I don't see a reason to break these into seperate permissions, considering the number of people permitted to do any editing would be quite small. Do you think we would?

and would we remove the option to re-upload a lookup table with changes - so we don't have to maintain changes in 2 places?

Yes, this is a good point, and one of the reasons why editing lookup tables it a little dicey for me, cos it's easy to mess up if you do it the wrong way. Lookup tables can be downloaded, then edited in bulk, then re-uploaded, but not sure what we do to prevent against silly mistakes (e.g. uploading new version of lookup table without first downloading current state)

CarlosNZ commented 2 years ago
  • When 2 users are trying to edit the same Lookup table how do we prevent someone overwriting others work?

If they're editing different records, then there's no problem. If they edit the same record, then the newest change will always persist. I don't see a problem with this though, as editing is going to be a pretty "senior" permission, with only a few people (maybe only 1) allowed. And they shouldn't just be making ad hoc changes based on personal whims, there would be some "official" reason to do so, so different users shouldn't have conflicting changes to make -- if they do, then some funny business is going down.

CarlosNZ commented 2 years ago

Also -- we should log any edits in the "activity_log" so we can always keep track of what's changed

nmadruga commented 2 years ago
  • Add a field to "outcome_display" table called "edit_permission_names", specifying what permissions are required to be allowed to edit outcome table data. Default for normal outcomes should be NONE (i.e. not editable at all), and for lookup_table displays can be "admin" (Format is same as existing "permission_names" field -- an array of permission names) (Time estimate: 1-2 hours )

Instead of using an array of permissinNames should we have another table (as we do for templatePermission) that stores a connectioin between the outcomes_display and the permission required to do the editing?

nmadruga commented 2 years ago

4. then when in "Outcome Detail" view (i.e. a single item), then each field can be made editable by clicking in it and changing the value.

I think it would be nicer if this editing can be done in the list as well. So, basically allowing the user to edit a line. Maybe we add an Edit button on the side and once clicked they can click on the cell to edit and then save changes (to be done per line)

nmadruga commented 2 years ago

Question: How do we handle row-level policy generation for new lookup tables (and outcome tables more generally too)?

Is the row-level generation required for lookuptables? I though the user that has permission to edit a lookuptable would be able to edit any row, not sure what do you meant by row-level in this case.

To allow adding which permission should be used for editing a new imported Lookup-table I think it would be good to add as a step during the import process. Maybe we can list all the available permissions existing (with some new especific type, i.e EDIT) and allow the user to select one for be managind editions in this lookup table. As you suyggested the Admin would be the most common case to be used. But perhaps they might want to use different permissions for different tables.

nmadruga commented 2 years ago

Meeting's minutes On 04/03/2022: