1229891 - [admin] Logs for Manage operations

Status: RESOLVED FIXED

(Webtools Graveyard :: Pontoon, defect, P3)

Comment 1

[Matjaz Horvat [:mathjazz]]

Could you please elaborate on this?

Comment 2

[YF (Yang)]

Logs for https://pontoon.mozilla.org/zh-CN/manage/ and other languages.

Comment 3

[Matjaz Horvat [:mathjazz]]

Oh, that makes sense.

Comment 4

[Francesco Lodolo [:flod]]

I feel more and more the need for this: a log of role changes for each user, and who initiated it.

Storing that information in the DB without a UI would already be a good start, so we can start collecting this information as soon as possible.

Comment 5

[Matjaz Horvat [:mathjazz]]

Hey flod, could you clarify what information in particular do you feel the need for and what's the use case behind it?

That would help specing the data model changes and reprioritize this bug.

Thanks!

Comment 6

[Francesco Lodolo [:flod]]

(In reply to Matjaz Horvat [:mathjazz] from comment #5)
> Hey flod, could you clarify what information in particular do you feel the
> need for and what's the use case behind it?

For example I'd like to know when a person became a manager (or translator), and who changed their role (was it an admin or another manager?). 

This information is useful to figure out the situation when there are multiple managers for one locale, or for locales that we are trying to reboot.

I know of at least one case of contributor who was promoted to translator, then downgraded to contributor again.

Comment 7

[Jarek Śmiejczak [:jotes]]

:flod :mathjazz

Do you have any other use-cases? From my point of view this looks like a simple Model for logs.
I can create small model with access via django admin interface.

Comment 8

[Francesco Lodolo [:flod]]

I think storing any role change for a user ("from role", "to role", "performed by"), not just the latest change, would solve any potential use case.

Comment 9

[Matjaz Horvat [:mathjazz]]

Jotes, feel free to assign the bug to yourself if you want to take it.

Comment 10

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/756

Comment 11

[Francesco Lodolo [:flod]]

Any update on this bug? It looks like if fell through the cracks (I understood it was basically done).

Comment 12

[Jarek Śmiejczak [:jotes]]

:flod

Sorry for the delay, After All Hands I somehow got stuck at fixing unit tests/adding docs and I probably needed a ping to wake up from my holiday slumber.

Anyway: the current state of my work: I have a couple of unit-tests to finish and still do some docs/code related fixes before I'll push it to review. PR should land near friday night/saturday right.
If everything will go smoothly with review, logging will land in next week on staging.

Also, I'm not sure if you were included in a discussion about fields which will be displayed in log.
Could you look at this screen: https://imgur.com/a/akhq4 and check/comment if current state is acceptable?

Comment 13

[Francesco Lodolo [:flod]]

I remember some discussions, but it was probably over IRC.

Action type: likely better "Removed", "Added".
Group: simply "Managers", "Translators".

It's missing information on the locale.

Comment 14

[Jarek Śmiejczak [:jotes]]

:flod

Currently, every group's name contains a locale's code e.g. on the screen-shot you see "an translators" - "Aragonese translators".
I can split that name into 2 fields, if that's not readable enough.

Comment 15

[Francesco Lodolo [:flod]]

I assumed that was a weird article… If that's the case, it should be clear enough with any other locale code.

Comment 16

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/795

Comment 17

[Axel Hecht [:Pike]]

Did you consider hooking this data up to django.contrib.admin.models.LogEntry?

Comment 18

[Matjaz Horvat [:mathjazz]]

Found this in the IRC logs:

Tuesday, October 24th, 2017, 10:19 AM
> jotes: adrian/mathjazz: https://bugzilla.mozilla.org/show_bug.cgi?id=1229891 what do you think about LogEntry https://github.com/django/django/blob/master/django/contrib/admin/models.py in this case?

> adrian: LogEntry doesn't seem to be the right tool for that. As far as I can tell, it's specific to django's needs of logging actions in the admin app. I worry that hijacking that might cause more problems than it solves
> adrian: I tried to see if there's an existing library out there that solves this problem but the few I found haven't been maintained since 2012

> jotes: adrian: I totally agree. I tried to google all those "log" libraries and from my professional experience, almost everyone ends up with "custom" model. I kinda tried to avoid that by reusing log-entry (reusability 9000)
> jotes: adrian: If you're fine with a small model for this task then I'll happily proceed to the implementation.

> adrian: jotes, I'm fine with making our own implementation, or using any library that is helping with that regard. I think django's LogEntry is not the right tool for that though

Comment 19

[[github robot]]

Commit pushed to master at https://github.com/mozilla/pontoon

https://github.com/mozilla/pontoon/commit/e9aec0b0425d5f285270f91d5ea682ca39f23cbb
Fix bug 1229891 Add logging of changes in permissions of users. (#795)

* Fix bug 1229891 Add logging of changes in permissions of users.

Every change in a contributor's permissions should will be stored in PermissionChangeset model.
Unfortunately, request.user isn't available for The Django signals and because of that the logic is implemented in views (which at first may look less intuitive).
Currently, there are 2 views that are able to modify a contributor's permissions - the first one is only accessible for admin/superusers and the second is a locale's "Permissions" tab.

* review fixes

* review fixes #2