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

Is wf_owner updated for non-marc records? Should it? #1584

Closed
fjorba opened this issue May 27, 2024 · 6 comments · May be fixed by #1695 or #1697
Closed

Is wf_owner updated for non-marc records? Should it? #1584

fjorba opened this issue May 27, 2024 · 6 comments · May be fixed by #1695 or #1697
Assignees

Comments

@fjorba
Copy link
Contributor

fjorba commented May 27, 2024

I've just discovered that in my Muscat instances, wf_owner is updated only for Marc records (publications, sources, people, institutions, etc), but not for standard_titles, standard_terms, places or liturgical_feasts, even after effectively updating records. I'm pretty sure that I haven't patched anything related to this, but may I ask if it happens also in your instances, please?

If not, would you agree that it should be updated?

I have seen that all models, Marc and non-Marc, have the same belongs_to :user, :foreign_key => "wf_owner" statement, so the reason for not being updated appears to be a bug.

MariaDB [traces_development]> select distinct wf_owner from publications;
+----------+
| wf_owner |
+----------+
|        1 |
|        0 |
|        7 |
+----------+
3 rows in set (4.566 sec)

MariaDB [traces_development]> select distinct wf_owner from people;
+----------+
| wf_owner |
+----------+
|        3 |
|        6 |
|        0 |
+----------+
3 rows in set (0.238 sec)

MariaDB [traces_development]> select distinct wf_owner from institutions;
+----------+
| wf_owner |
+----------+
|        1 |
|        0 |
+----------+
2 rows in set (0.073 sec)

MariaDB [traces_development]> select distinct wf_owner from standard_titles;
+----------+
| wf_owner |
+----------+
|        0 |
+----------+
1 row in set (0.015 sec)

MariaDB [traces_development]> select distinct wf_owner from standard_terms;
+----------+
| wf_owner |
+----------+
|        0 |
+----------+
1 row in set (0.008 sec)

MariaDB [traces_development]> select distinct wf_owner from places;
+----------+
| wf_owner |
+----------+
|        0 |
+----------+
1 row in set (0.001 sec)

MariaDB [traces_development]> select distinct wf_owner from liturgical_feasts;
+----------+
| wf_owner |
+----------+
|        0 |
+----------+
1 row in set (0.070 sec)

@fjorba
Copy link
Contributor Author

fjorba commented May 27, 2024

After some more tests, I understand better the current behaviour: a new non-Marc record, either created via the new button or via a bibliographic, effectively creates the wf_owner field, but this field is never updated again.

We are interested in updating it, so we know who did this modification.

@xhero
Copy link
Contributor

xhero commented May 27, 2024

I think we just never implemented it, we never really used this functionality (since only editors can touch auth files), but if you want to patch it go ahead!

fjorba added a commit to fjorba/muscat that referenced this issue May 28, 2024
Make this field visible in the display page, and update it using current
user field.  After uptading, redirect to the display page, because if
redirects to the list, the modified record is lost and not easily
accessible, except via the browser history.

Closes rism-digital#1584
fjorba added a commit to fjorba/muscat that referenced this issue Dec 16, 2024
Currently, after editing those records, Muscat takes you to the record list,
without allowing to double-check if the editing was right.

Partially closes rism-digital#1584
@fjorba
Copy link
Contributor Author

fjorba commented Dec 16, 2024

The proposal is not accepted, as seen in the discussion at #1589, so I'm closing it.

@fjorba fjorba closed this as completed Dec 16, 2024
@xhero
Copy link
Contributor

xhero commented Dec 16, 2024

The problem in #1589 is not really using wf_owner, it is that it would be changed to the user logged in every time the record is saved! I think wf_owner should be and "owner" that stays the same unless explicitly changed by an editor. To know the history of who edits a record, :versions can be used, as each time a record is saved an old version with the name of the user is saved.

So for me: ok to use wf_owner everywhere, but it should not be changed each time the record is saved.

@fjorba
Copy link
Contributor Author

fjorba commented Dec 17, 2024

I understand. The problem with my PR is that it was doing three different things: making wf_owner always visible in all those non-marc authority record, updating it at each update, and redirecting to the full record instead to the list. So it caused confusion and discussion, my fault. In my defense, I can say that some PR take so much time to be incorporated, sometimes accepted ( #1363), others without comments (#1651), that I wanted to rush a little bit this incorporation, knowing, of course, that it is better not to mix different fixes in the same patch. My pile of pending contributions is growing locally in our servers, but they depend on the pending ones to be accepted (or not!).

Anyway, #1695 is in the queue, and I'll create another one for showing wf_owner for those non-marc authority records, without updating each time the record is saved.

@fjorba
Copy link
Contributor Author

fjorba commented Dec 17, 2024

Moreover, those non-marc authority records do not have versions. So we thought that a simple way to know who did the last edit was updating the owner. It is acceptable in our environment, but it may be not in yours, of course.

fjorba added a commit to fjorba/muscat that referenced this issue Dec 17, 2024
Those records already had an ownwer, but it was not visible on the screen.

Fixes rism-digital#1584
fjorba added a commit to fjorba/muscat that referenced this issue Dec 17, 2024
Of all non-marc authority records, place is the only that doesn't display
wf_stage, although the field exists in the database.  Make it visible, so it
is homogeneous with the others.

Part of rism-digital#1584
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment