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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log all actions for admins-MVP #2603

Closed
mrcasals opened this Issue Jan 30, 2018 · 40 comments

Comments

Projects
None yet
5 participants
@mrcasals
Collaborator

mrcasals commented Jan 30, 2018

This is a Feature Proposal

馃帺 Description

In order to improve control on what other admins are doing, we want all actions done by admins to be logged and shown in a log feed in the admin section.

For each action, we want to see:

  • user who performed the action
  • date and time of the action performed
  • the URL where the change was made
  • if applicable, the field that has been modified and the previous and the new value

We also need to decide if this info is only accessible for organization admins, or space admins can view this too. Update: on #2611 it was decided to start showing the admin logs only to organization admins.

This issue needs design. Design has been provided and accepted by @decidim/product.

馃搶 Related issues

馃敤 Related PR

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Jan 30, 2018

@decidim/product I have a few questions about this that were not answered in the last meeting.

We need to decide (heh) who has access to this log. For simplicity, I'll start working on this considering only organization admins have access to this. I think allowing space admins to see this log raises a question: should space admins see data only for their assigned spaces, o should they see everything? Since controlling this is much harder, I'll ignore this until you guys decide what to do.

What happens when the resource of a given action is deleted? Should we remove all actions related to that resource, or should we keep showing them somehow?

Just to be sure, the modified fields will only be shown when an admin updates a resource, not when they create it. Is that OK?

And, again, I will need design for this.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Jan 30, 2018

I'm thinking about showing this text for each action:

%{user name} %{action} %{resource title} (%{resource type}) in %{space title} (%{space type})

Space title and resource title would be links to the resources. User name could be a link too. An example of text, with the variables set, could be:

Jane Austen (@janeausten) created "Regular meetup" (meeting) in "Book Cycle" (participatory process)

Thoughts?

@arnaumonty

This comment has been minimized.

Member

arnaumonty commented Jan 30, 2018

We need to decide (heh) who has access to this log. For simplicity, I'll start working on this considering only organization admins have access to this. I think allowing space admins to see this log raises a question: should space admins see data only for their assigned spaces, o should they see everything? Since controlling this is much harder, I'll ignore this until you guys decide what to do.

For a moment I would say just give access to organization admins, not space admins.

What happens when the resource of a given action is deleted? Should we remove all actions related to that resource, or should we keep showing them somehow?

We should keep showing the previous actions.

Just to be sure, the modified fields will only be shown when an admin updates a resource, not when they create it. Is that OK?

Ok

And, again, I will need design for this.

Sure. We should involve @decidim/lot-px here. Just to clarify this page should be shown on the main page of admin board but we have to take into account that in this page-board in a soon future we will add other statistics of decidim.

%{user role} %{user name} %{action} %{resource title} (%{resource type}) in %{space title} (%{space type})

I miss the date of the action. I will put first user name and then user role.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Jan 30, 2018

@arnaumonty perfect, thanks!

@mrcasals mrcasals referenced this issue Jan 30, 2018

Merged

Add admin logs codebase #2604

9 of 9 tasks complete
@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Jan 31, 2018

A thing to consider: I can only control what the components in this repo do. Actions performed by admins in external components won't appear in the admin log unless they apply the same changes I'm doing.

If I didn't have to track what fields have changed and who has performed the action, then I could do this mostly automatically, I guess, but since I need to save this data I cannot do it magically, and some changes need to be applied.

I think it's something @decidim/product needs to be aware of, just in case.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Jan 31, 2018

Ah, also: tracking all this data will create a lot of records in the database (a couple of database rows per action).

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 5, 2018

@decidim/product just as with notifications, this has to be an EPIC, since I have to manually log the actions, for each component and resource.

@andreslucena

This comment has been minimized.

Member

andreslucena commented Feb 5, 2018

A thing to consider: I can only control what the components in this repo do. Actions performed by admins in external components won't appear in the admin log unless they apply the same changes I'm doing.

Ok, on that case is important to have documented (on docs/) the APIs that the components consume.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 5, 2018

@andreslucena I'm writing docs on how to log action while I develop (see the PR), if you want to take a look 馃槃

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 6, 2018

@decidim/product PR has some screenshots:

#2604

Remember that this issue needs design, and my design skills are very limited, so that's all I can achieve right now.

@javierarce

This comment has been minimized.

javierarce commented Feb 6, 2018

Yes, I'm on it :) I'll submit a design this morning.

@javierarce

This comment has been minimized.

javierarce commented Feb 6, 2018

Here's my proposal:

  • The idea is use a different color for the actor, object, and location of the entry.
  • The extra information (like the username or in case the number of characters excedes a certain threshold and we need to truncate the string) could be hidden and revealed in a tooltip (a custom one or the HTML 'alt').
  • Destructive entries have a red background.

log

https://marvelapp.com/7h98ada

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 6, 2018

@javierarce note that the description of this issue says we need to show what attributes changed, and old and new values. Since this can be a lot of changes, what do you think about adding a "show" view for individual log entries where we show this data?

/cc @decidim/product

@javierarce

This comment has been minimized.

javierarce commented Feb 6, 2018

Could we also show the previous attribute on a tooltip? Or would that be too much information?

Or maybe we could have an extra line in that kind of entries showing the previous value.

log_2

Not that I don't like what you propose, but at this stage I think it would be better if we could avoid having to click to reveal information.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 6, 2018

@javierarce yes, but we might not know how many attributes have been changed, and how long was the value (maybe it was just true/false, or was a long text)

@javierarce

This comment has been minimized.

javierarce commented Feb 6, 2018

Ok, in that case let's add a show view as you propose where we can show texts of any length. I'll add a design for that too.

@javierarce

This comment has been minimized.

javierarce commented Feb 6, 2018

What about having something like this:

log entry

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 6, 2018

Yeah, this is somewhat similar to what we currently have in result versions:

#2206 (see screenshots)

(I pulled the design off my hat, I know it could be improved but...)

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 7, 2018

@decidim/product I have a working MVP but I would like to wait for design before finishing it, can your review the design so we can move to the actual HTML part and I can keep working on this? 馃槃

Thanks!

@javierarce

This comment has been minimized.

javierarce commented Feb 7, 2018

I'd like to propose another solution to show the current and previous value. Instead of having a show view, we could just have a toggle, to avoid having to go back and forth:

log toggle
log toggle 2

https://marvelapp.com/7h98ada/screen/38207019

@arnaumonty

This comment has been minimized.

Member

arnaumonty commented Feb 8, 2018

@javierarce this last proposal looks good to me.
@mrcasals I've check the MVP and it looks good too. I'm not sure how to proceed to apply the new design.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 8, 2018

@arnaumonty I'd say @decidim/lot-px can create a PR to update this design so I can reuse it. Maybe the design app does not have the admin design (/cc @josepjaume), but maybe we can try to use https://github.com/decidim/design-admin so that we are not so blocked?

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 12, 2018

Update: on #2611 it was decided to start showing the admin logs only to organization admins.

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 12, 2018

Update: I'll start adding the layout for this /cc @decidim/lot-px I'll send another PR to add the layout, and then I'll move the code from there to this PR 馃槃

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 12, 2018

@decidim/lot-px can I have an icon for the icon sidebar? They are custom, right?

@javierarce

This comment has been minimized.

javierarce commented Feb 12, 2018

@mrcasals it seem most of them come from https://useiconic.com/open. Let me look for a good icon or design a custom one.

@javierarce

This comment has been minimized.

javierarce commented Feb 12, 2018

What about using the one from "list"?

screen shot 2018-02-12 at 13 00 11

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 12, 2018

@javierarce oh, sorry! Yes, they come from iconic, the list icon looks good to me! Sorry for disturbing and thanks! 馃槃

@javierarce

This comment has been minimized.

javierarce commented Feb 12, 2018

No problem! :)

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 12, 2018

@javierarce what happens when a log is an update, and more than one field has been updated? How is each field displayed?

I'm thinking of adapting the design from #2603 (comment), since the show page allows for more customization, and creating a table for each attribute modified. What do you think? This is the same solution we used for the results traceability design (#2206).

Does this work for you? Or do you have any other idea?

@javierarce

This comment has been minimized.

javierarce commented Feb 12, 2018

Yes, your idea makes total sense, let's go for it. I'll update the design to support that case.

@javierarce

This comment has been minimized.

javierarce commented Feb 12, 2018

This is my solution for the update with multiple fields:

log with multiple fields

In order to avoid showing a very tall element with all the updated fields, I propose with set a fixed height and make the content inside scrollable:

log with multiple fields scroll

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 12, 2018

@decidim/product does this work for you? 馃槃

@andreslucena

This comment has been minimized.

Member

andreslucena commented Feb 12, 2018

@decidim/product does this work for you? 馃槃

馃憤

@Xfolchf Xfolchf added this to the CdP2 milestone Feb 12, 2018

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 13, 2018

PR #2715 implements the design into the layout, but now I see I forgot to show the author username on a tip bubble, I'll add it!

Update: tooltip added

@Xfolchf Xfolchf changed the title from Log all actions for admins to Log all actions for admins-MVP Feb 15, 2018

@wafflebot wafflebot bot removed the status: layout label Feb 16, 2018

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 19, 2018

@javierarce @decidim/product while developing this and trying out on my machine, I came to the situation where I created a resource and immediately deleted it (without updating it in the middle). The current layout only shows the attribute diff when the resource is updated.

Should we show the diff when creating the resource too?

@arnaumonty

This comment has been minimized.

Member

arnaumonty commented Feb 19, 2018

@mrcasals yes we should show the diff.

@javierarce

This comment has been minimized.

javierarce commented Feb 19, 2018

I'm probably missing something here, but wouldn't the log show a "create" and then a "delete" line? What would diff mean in this case?

@mrcasals

This comment has been minimized.

Collaborator

mrcasals commented Feb 19, 2018

@javierarce Yes, it would show those lines, but the log would not show what attributes has the resource on creation.

For this, I propose adapting the current design this way:

It's the same as for updates, but not showing the rows with the previous values (as there would be no previous values on creation)

@javierarce

This comment has been minimized.

javierarce commented Feb 19, 2018

Ah, I see, ok!

@mrcasals mrcasals referenced this issue Feb 19, 2018

Merged

Show logs diff on resource creation #2759

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment