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

Forgiveness #79

Merged
merged 25 commits into from
Nov 16, 2018
Merged

Forgiveness #79

merged 25 commits into from
Nov 16, 2018

Conversation

krisfield
Copy link
Collaborator

Adds special page:ClearPendingReviews to allow for users with userright:clearreview the ability to clear pending reviews without the need of using the terminal

@jamesmontalvo3
Copy link
Contributor

WatchAnalytics has never had a release before. I just cut an arbitrary 0.1.0 and am in the process of making Meza track that version. Once that's part of Meza's master and 32.x branches, and there's a 32.2.2 version released, then we can look into this change and #77.

FYI, I also made Meza track specific versions of every other extension EMW maintains.

Copy link
Contributor

@jamesmontalvo3 jamesmontalvo3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting things listed are just suggestions. I think they'll make the special page better, but I'll merge without them. The change versus keyup/keydown thing I think is critical.

One other thing that I'm not sure how to comment on in code below: have you considered what may cause a request to timeout? I did some wider time ranges and preview took a long time. Of course this isn't intended for wide time ranges, and I think you hope to add some validation to prevent such ranges at some point, but I'm not sure if you'd get similar long response times for a short time range but with a large number of reviews... e.g. if a ReplaceText edited 1000s of pages in a short period of time. Could that cause a timeout? And could that timeout be prevented by limiting the number of pages/people shown in the preview? Something like "this preview would exceed X pages and cannot be displayed. You can choose to blindly clear the reviews anyway at your own peril, but it is recommended you further limit your query instead."

@krisfield
Copy link
Collaborator Author

krisfield commented Nov 15, 2018

I incorporated all of your feedback except for the timeout limit, trying to think of best way to handle that. I don't think that is a required change before merging this but I will work it in the future.

I also added Hooks to the Preview and Clear Pages actions so we could tie in slackbots or whatever we want with those

@jamesmontalvo3 jamesmontalvo3 merged commit df3874d into master Nov 16, 2018
@jamesmontalvo3 jamesmontalvo3 deleted the forgiveness branch November 16, 2018 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants