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

Improve logging when changing curator rights #25

Closed
2 tasks done
mkroon1 opened this issue Feb 11, 2016 · 4 comments
Closed
2 tasks done

Improve logging when changing curator rights #25

mkroon1 opened this issue Feb 11, 2016 · 4 comments
Assignees

Comments

@mkroon1
Copy link
Collaborator

mkroon1 commented Feb 11, 2016

When changing curators or curator's rights (genes/DMD?authorize), LOVD logs that the form is submitted, but not what has changed. It would be useful to change the lovd_writeLog() call to include the difference, so you can see in the logs if a user was removed as curator, demoted to collaborator, collaborators who became curators, or users added as curators.

  • First make a plan on how this log entry would look for all possible situations, so we can discuss
  • Then implement by creating something like $sAuthorizeDiff and including that in the lovd_writeLog() function call
@mkroon1 mkroon1 self-assigned this Feb 11, 2016
@mkroon1
Copy link
Collaborator Author

mkroon1 commented Feb 12, 2016

While managing curators for a gene, one can:

  • give edit rights
  • show/hide
  • change the order of shown curators

I propose to let the log message show the new order of curators with
edit/show attributes and whether the attributes are changed (see
example below).

I think showing the difference in order of curators would make the log
message convoluted. If one wants to see the difference in order, he can
go to the logs page and filter the entry field for "Updated" and
"$geneID".

@ifokkema, please let me know what you think.

Example log message:

Updated curator list for the $geneID gene:
1. Name1, ID1, allow_edit=true, show=true, unchanged
2. Name2, ID2, allow_edit=true, show=false, set:show=false
3. Name3, ID3, allow_edit=false, show=false, set:allow_edit=false + set:show=false

@ifokkema
Copy link
Member

I agree it would be good to explain the difference and what changed, but this seems like a lot of text already, especially if almost nothing changed. Wouldn't it be better to just show what's been changed? In your example, I think you mean to say two curators got hidden, right? So show should be "true" for IDs 2 and 3, no?

How about:

Updated curator list for the $sGeneID gene:
Name2 got hidden from view.
Name3 got hidden from view and was demoted to collaborator.
Order is now: Name1, Name2, Name3.

with all info removed that didn't change. Wording can be different. I find this easier to read, especially when little information changed.
If we worry about users with different names, perhaps we can create some sort of tag; "{User:00001:Ivo F.A.C. Fokkema}" could then be turned into a link by prepareData(), so we don't need to show the ID, while at the same time allow the user to navigate to the user in question.

If you agree, we can write up all possible log entry lines, and then implement.

@mkroon1
Copy link
Collaborator Author

mkroon1 commented Feb 16, 2016

I agree. I guess we should also mention added/removed curators. An example with all possible changes (I believe) is then:

Updated curator list for the $sGeneID gene:
Added name1 (#ID1)
Removed name2 (#ID2)
Hidden name3 (#ID3)
Unhidden name4 (#ID4)
Order is now: name1 (#ID1), name4 (ID4), name3 (#ID3), name5 (#ID5)

In my opinion extra markup with links is not necessary, it may even hinder readability when looking at the logs directly in the database.

@ifokkema
Copy link
Member

OK, looks good!
I never look at the logs in the DB directly, but you're right, we might not need the markup. See this block here, if we use the format user #00001 (Name), we should be good already if we just add the event to the options here.
We could also make the format like you proposed: Name (#00001), but then we'll need to adapt the code in the log's prepareData().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants