-
Notifications
You must be signed in to change notification settings - Fork 319
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
Admin Feature: Show absolute flag count #641
Comments
arggh wrong issue sorry... moved. |
@sizzlemctwizzle |
@TimidScript In summary not every feature you are going to want will be done client side due to server constraints and other factors yet to be discussed. This particular feature should not be available to client side .user.js in my current opinion.
-1 unless we encounter an issue with out of sync operations... #77 and potentially #285 are better entry points for what the server shares and your comment is an expansion on #646. |
Do we really need a count sent to the views here? The reason why I ask is when #643 is implemented with showing everyone (allowed of course) who flagged a script on script homepages (and elsewhere)... it's easy enough to count the visible user names that show up in Admin Tools... unless you meant showing the real count in Mod Tools? |
Btw here is a draft of what #643 currently looks like in a users script list: ... still have other locations to go but on a script homepage you would see them in the Admin Tools... and one could just count how many show up instead of taking up more screen real estate for a number count. :) (the flag icon, or other, is temporary since we don't have reasons yet but that is in the plan soon) |
It's unnecessary to show count if we just list the names of users who Mods shouldn't see the absolute flag count.
|
That looks good.
|
* Base working concept * Eventual showing of the reason in respective views... currently set to a horizontal ellipsis if absent... see additional notes below Applies to OpenUserJS#643 and OpenUserJS#641 NOTES: * Test with QSP `?flagged=true` for lists and Admin or better privileges on script homepage and user homepage with logged in and logged out... there are at least 6 routes *(4 with QSP and 2 without but all elevated to Admin+)* needed to test against * If for some reason some user is flagged but there aren't any actual flags it will show up as no user and no reason e.g. Flagged column will be empty for those mustache opts * Using series in some of the *async* related calls to hopefully ensure natural ordered flags * **NOT COMPACTED** for clarity as well as for strict *async* usage * One bug fix in a view with colspan * One restoration in `.../modelParser.js` * Some class changes in a view TODO: *(mitigation)* * Needs reason attached to flagging route ... separate issue to be created and milestoned near OpenUserJS#485 and OpenUserJS#262 and similarly done in OpenUserJS#513
* Standardize on `flagList` since `flagged` is already in use * Some compaction of the code ... backed out a few changes in prior commit... added a master asynchronous function to the flag library * One bug fix with `modelParser` to acccomodate prior commit reversion **NOTES**: * This should be functionally equivalent to the last commit feature wise and a lot easier to maintain. An example of this is the dynamic field name change in this commit... easily changeable in case someone doesn't like that name but should be okay for now. Applies to OpenUserJS#643 and indirectly OpenUserJS#641
Just a FYI we still probably need to store the absolute counts in the the Script and User models (and others probably) to affect #643 a bit. I would like to see any flag on the users script list regardless of the "critical" algorithm we have that show up in moderation... e.g. if one shows up there but has tons of upvotes we can still see if the author has a bunch of nominal flags and visa versa we can detect more easily if a user is abusing the flagging system (I've already seen this). So we still have to include this issue for that capability. Shooting for |
It doesn't matter if script.flagged is false or not. You can still fetch
|
Admins+ would be able to detect abuse of a particular user whether it be by an author or on an author... but only needed on the this route.
In the existing PR it only show "critical mass" flags everywhere because of the way the list is whittled down e.g. not an issue of the PR but of how we get the lists. You can see this by temporarily commenting out this line... this shows every script whether flagged or not...and every flag instead of just critical flags with how we calculate I've already done the preliminary tracking in our code to find this intel out as you can tell... unless you have a better idea? |
Don't forget to add the QSP of Does this specific example make more sense? |
More noise... also if you goto http://localhost:8080/scripts/MAX30/TopAndDownButtonsEverywhere (pro) with just the plain PR you will always see the single flag (provided it's not cleared) but the upvotes mask the flag in any list we return... this is because it's not a filtered list of scripts and isn't privy to that commented out line. If we don't filter the lists on a different variable with a users script list then we (admins and up) are in the dark for flag bombing until it becomes critical. We can still keep the critical flags view on the other routes because we don't actually care when going to the moderation route but someday someone will strafe flag and it will be an AuthAs mess too... which is better than manually querying the DB but still a lot of work... if I were to see that the referenced user was attacked by a single author on all of his scripts in the critical moderation route, I could easily check all the attacked authors scripts and make a better decision if the abusers account should be axed or just ignored. The code can easily be done with: // 2nd conditional test doesn't exist yet but
// I'm sure a detectable method can be available in `aOptions`
// even if it's another QSP to show every flag
if (isAdmin && isUsersScriptRoute) {
aModelListQuery.and({ flaggedCount: { $gt: 0 } }); // whittle down to all flags
} else {
aModelListQuery.and({ flags: { $gt: 0 } }); // whittle down to critical flags
} |
a-ha it is already set with if (aOptions.isAdmin && aOptions.isUserScriptListPage) {
aModelListQuery.and({ flaggedCount: { $gt: 0 } }); // whittle down to all flags ... but `flaggedCount` doesn't exist yet and is this issue #641
} else {
aModelListQuery.and({ flags: { $gt: 0 } }); // whittle down to critical flags
} |
Another, perhaps less desirable, option for #641 here is to just show every script whether flagged or not e.g. if (aOptions.isAdmin && aOptions.isUserScriptListPage) {
// Do nothing but show everything found for that users scripts if flagged or not
} else {
aModelListQuery.and({ flags: { $gt: 0 } }); // whittle down to critical flags
} The logic could be compacted but this works too although if an author has 50 pages of scripts it's going to get interesting since it's not whittling down to any flags. |
So summarizing:
/end of this noise for a time to ponder |
* This is option two Applies to OpenUserJS#641
* Denote that this is only searching for critical flags instead of all flags. Applies to OpenUserJS#641
Option 1 would not accomplish this on script lists anywhere... however going to the actual script home page itself would divulge this due to #643 Option 2 would accomplish this on user script lists only... however Flagged Script and Flagged Users (moderation) would still hide this Option 3 would accomplish all of this however it would hide flag bombing of high upvoted scripts. (the inverse of what @sizzlemctwizzle mentioned and what I sometimes look for) UGGH! Any other ideas? |
Hmmm perhaps... 4a) QSP of |
The next step is to do some DB migration updates. Would you mind if I converted this, this, and this to an Object? (presuming the query will allow this... it should I think since I did that similarly here) ... so it would be I have to do a DB migration update at this point anyhow and may as well fix this so it's readable and you wouldn't have to come up with another identifier name for me (same issue as yesterday where I've gone between a bunch of names ;). e.g. create the field name properly (step A), (step B) zip around to the absolute flag count and save that into each model for each script and user. (know where to do this btw from #643 :) Btw last commit for this issue is on production if you want to give it whirl... just note that Absolute isn't working yet because of what I'm asking in this comment. :) |
Yeah I'm fine with your proposed migration. |
@sizzlemctwizzle |
* Project version bump... e.g. a "hump" in the DB... going back will require an earlier commit HEAD and an earlier DB * Change everything affected * Tiny bit of STYLEGUIDE.md compliance * Error trap flag user... if currently seen means that a user migration didn't occur **NOTES** More to go... Applies to OpenUserJS#641
DB Migrated and VPS updated... so far so good... approximate time was ~8 minutes... could have shaved a minute or two off but still asynchronous in some areas and wanted to ensure that each model was migrated. |
Next step (step B) is to accumulate the Absolutes and modify the aggregation function... need a bit to do that. Running with current for a bit to iron any fallout. |
* One missed label from OpenUserJS#643 in Mod Tools. * More open and closed flag changes for symmetry **NOTES**: * Still to go... reset the absolute DB values for things already flagged/unflagged Applies to OpenUserJS#641
At everyone whois Admin+ ... I manually fixed Hopefully later this evening after food break I'll fix all the records of currently flagged items... then the list will grOW! |
Done with DB migration into @sizzlemctwizzle |
@sizzlemctwizzle Cleaned up dev a bit too... still need those flags in for the next issue of adding the reason so please don't clear them yet. |
* Code point ref https://github.com/OpenUserJs/OpenUserJS.org/blob/7b3c673ddc020d31617942bc8ba47e219c20a553/controllers/script.js#L540 **NOTES** * This causes `flags.absolute` to be negative which should never happen... didn't realize we were using zero so didn't include it with the ES5 code. Post fix for OpenUserJS#641
* This causes `flags.absolute` to be negative which should never happen **NOTE** * ES6 routine could still be used however logic would have to be post change... so removed that comment note Post fix for OpenUserJS#641
* This time positive increment when unvoting Post fix for OpenUserJS#641
* When promoting/demoting a User future flagging/unflagging can wreak havoc on critical *(relative)* flags properties. * Project bump since requires a bit of migration * Needs followup testing with OpenUserJS#1109 Applies to OpenUserJS#126 and post OpenUserJS#641
Admins need to be able to see the absolute flag count (not the count that is negated by upvotes and karma). Show absolute flag count on script page, user page, script listing and user listing pages. This would allow admin to see potentially malicious scripts that are masked by upvotes, or the rare popular script turned bad.
The text was updated successfully, but these errors were encountered: