rism-digital / muscat

🗂️ A Rails application for the inventory of handwritten and printed music scores
http://muscat-project.org
34 stars 16 forks source link

Two new fields: wf_edited_by and wf_event? #1634

Open fjorba opened 5 days ago

fjorba commented 5 days ago

Most of Muscat main tables have workflow (wf_*) fields like:

I tried to understand how paper_trail works, and how it fits into Muscat (as like any other Rails application), and I learned that paper_trail "stores the pre-change version of the model" (https://github.com/paper-trail-gem/paper_trail?tab=readme-ov-file#1c-basic-usage), but not the current one, that lives in standard Rails table. Paper_trail uses two fields to keep who did the last change (whodunnit) and which event created this last change (event).

With this knowledge, many months ago we were able to create an export and import procedure to migrate all our record history from Invenio into Muscat (currently 4,601,595 versions for our largest database).

However, as we are testing our Muscat instances, we noticed that there is no fields to store the equivalent of the the whodunnit or event fields into the current version of the record. Those fields should be the ones that would mutate into paper_trail versions table when a new version of a record is updated (whodunnit probably should store the text identification of the user). Otherwise, we cannot know who and how the record was last modified, and audit it.

So, we are interested in two new fields, and I'd like to ask whether this need could be shared, and part of standard Muscat:

During the last weeks I have done some ugly hacks to overcome this limitation without altering Muscat database schema, but the more it goes, the more I feel that this is fragile and probably the solution could be solved in Muscat itself.

Would you accept patches and migrations to initially create the fields? Updating them (should be automatic) and displaying them would be a second phase.

fjorba commented 10 hours ago

Let me make ask it in another way: in your Muscat instance at RISM, can you know who did the last modification of a record? (Or which program, if it was an automatic correction or update.) And the list of all people who modified it (and when and what, of course)? Would you be interested in this information?

xhero commented 3 hours ago

@jenniferward @lpugin What do you think? In theory we can use the versions to know who made a snapshot, but not all events are always snapshotted.

ahankinson commented 3 hours ago

wf_event is similar to the "commit message" idea we've been throwing around...

xhero commented 3 hours ago

true, it could have a default value or some message users can add

lpugin commented 3 hours ago

Yes, I think this is a similar idea. It should be also in the version history, maybe simply be using it int the versions.event column?

I agree we need a default message, because otherwise people just put subjective information (e.g., "minor changes")

fjorba commented 2 hours ago

Not really, the wf_event should be automatic and, as we have been using are, for example:

Except for the last one, all they also have the person (user id) who has edited or merged the records. It is not needed to explain it, like a git commit message, it is automatic.

I have it working in my instances, but reusing unused db fields. I'd prefer to have them right.

lpugin commented 2 hours ago

A problem we had when storing the user as id in versions was when a user would eventually be deleted. That might be an issue with wf_edited_by too.

fjorba commented 2 hours ago

That's why I proposed that in the versions table the whodunnit field should mutate into a text version of this user. We store the email address.

However, probably a better policy would be to never delete users, just mark them as deleted.

fjorba commented 1 hour ago

OTOH, the problem of deleted users also happens now with wf_owner, that keeps the user id.

lpugin commented 43 minutes ago

Yes, but it makes sense to require for the owner of a record to be changed before you can deleted a user.