Skip to content

List PaperTrail versions per event#628

Merged
erdgeist merged 6 commits intomasterfrom
erdgeist-event-history
Nov 6, 2019
Merged

List PaperTrail versions per event#628
erdgeist merged 6 commits intomasterfrom
erdgeist-event-history

Conversation

@erdgeist
Copy link
Contributor

@erdgeist erdgeist commented Nov 3, 2019

This PR adds a "history" tab to the events view, listing all versions of the event and associated events.

It also cleans up the tabs on event views, so I didn't have to manually add the history tab to each of the others.

A review would be quite appreciated.

@erdgeist erdgeist changed the title List PaperTrail versions per conference List PaperTrail versions per event Nov 3, 2019
@erdgeist erdgeist closed this Nov 3, 2019
@erdgeist erdgeist reopened this Nov 3, 2019
@elad-eyal
Copy link
Collaborator

Some comments after reading the code (didn't try it yet)

  • Did you forget to include history.html.haml ?
  • I think the history view should exclude the review and feedback models (i.e. EventRating, EverentReviewAverage, ... ) for two reasons. (1) I think they will spam the list (2) it's a way to circumvent pundit authorization to see this data.
  • I propose to move the tab order (from left to right) to be: Event details, people, history, Rating, Feedback
  • Related bug you might want to fix while at it: Adding/removing/changing classifier for an event is not stored in papertrail.

@erdgeist
Copy link
Contributor Author

erdgeist commented Nov 3, 2019

  • Added the template.
  • As the history is meant for coordinator/organizer only, anyway, I think it should be limited to those users in the first place. Leaving the spamming issue. I don't consider ratings spam. This is exactly the kind of changes I'd expect in this place. So basically we only need to exclude feedback. That could be handled by an extra :verbose parameter.
  • Changed the tab order and, you're right, looks more natural
  • Will look into adding PaperTrail to classifiers in another PR

@elad-eyal
Copy link
Collaborator

@erdgeist I would think you'd want reviewers to see the history of the events they're reviewing, but not other events. I don't think it's important to track old review scores (on the contrary maybe). Anyway I'm not sure the reviews are integrated to paper-trail completely anyway.

@erdgeist
Copy link
Contributor Author

erdgeist commented Nov 5, 2019

I see no easy way to exclude Ratings and Feedback. Updates types are saved as yaml key/value pairs deep in the PaperTrail version object with no efficient way to filter for them.

That being said, I actually didn't find it distracting seeing these verbose updates in a live conference, as they do happen at very specific times in an event's life cycle, making it actually pleasing to follow it.

@erdgeist erdgeist merged commit 0e0352d into master Nov 6, 2019
@elad-eyal elad-eyal mentioned this pull request Nov 6, 2019
@erdgeist erdgeist deleted the erdgeist-event-history branch November 9, 2019 14:46
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

Successfully merging this pull request may close these issues.

2 participants