OpenUserJS / OpenUserJS.org

The home of FOSS user scripts.
https://openuserjs.org/
GNU General Public License v3.0
846 stars 301 forks source link

Admin Feature: Show absolute flag count #641

Closed sizzlemctwizzle closed 8 years ago

sizzlemctwizzle commented 9 years ago

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.

Martii commented 9 years ago

arggh wrong issue sorry... moved.

TimidScript commented 9 years ago

@sizzlemctwizzle OUJS-1 does it in script page, only. You should consider including the metadata of the script within nodes attributes in the html listing. That way, it allows third party scripts can use it and alter the page accordingly.

Martii commented 9 years ago

@TimidScript I think there is some confusion here... this issue is for administration purposes only and not to be handled in a .user.js. Very limited things should be handled in .user.js especially when it comes to management. You'll notice I only have two scripts so far and those are unit tests for Ace and the meta routine e.g. this isn't like RoR with USO where everything had to be done in a .user.js because there was a closed source gap and a learning gap due to language differences. This projects code is in JavaScript (ES5.x) and usually should be done within the existing code base.

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.

You should consider including the metadata of the script within nodes attributes in the html listing.

-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.

Martii commented 8 years ago

@sizzlemctwizzle

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?

Martii commented 8 years ago

Btw here is a draft of what #643 currently looks like in a users script list:

issue-643userscriptlistdraft1 opt

... 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)

sizzlemctwizzle commented 8 years ago

It's unnecessary to show count if we just list the names of users who flagged the script for admins.

Mods shouldn't see the absolute flag count. On Oct 13, 2015 9:46 PM, "Marti Martz" notifications@github.com wrote:

@sizzlemctwizzle https://github.com/sizzlemctwizzle

Do we really need a count sent to the views here?

The reason why I ask is when #643 https://github.com/OpenUserJs/OpenUserJS.org/issues/643 is implemented with showing everyone 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?

— Reply to this email directly or view it on GitHub https://github.com/OpenUserJs/OpenUserJS.org/issues/641#issuecomment-147908805 .

sizzlemctwizzle commented 8 years ago

That looks good. On Oct 13, 2015 10:01 PM, "Marti Martz" notifications@github.com wrote:

Btw here is a draft of what #643 https://github.com/OpenUserJs/OpenUserJS.org/issues/643 currently looks like in a users script list:

[image: issue-643userscriptlistdraft1 opt] https://cloud.githubusercontent.com/assets/114709/10473859/5fd7a9ec-71ed-11e5-8333-0375e4949471.png

... 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. :)

— Reply to this email directly or view it on GitHub https://github.com/OpenUserJs/OpenUserJS.org/issues/641#issuecomment-147910781 .

Martii commented 8 years ago

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 flaggedCount for the model field name... still pondering (any idears?). Ideally (current) flagged and flags would make more sense with ~"karma" attached somewhere but extending the identifier out makes it more specific I guess.

sizzlemctwizzle commented 8 years ago

It doesn't matter if script.flagged is false or not. You can still fetch all of the flag records for a script. I don't see how storing the number of the records would help us. On Oct 17, 2015 12:02 AM, "Marti Martz" notifications@github.com wrote:

Just a FYI we still probably need to store the absolute counts in the the Script and User models to affect #643 https://github.com/OpenUserJs/OpenUserJS.org/issues/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.

— Reply to this email directly or view it on GitHub https://github.com/OpenUserJs/OpenUserJS.org/issues/641#issuecomment-148886176 .

Martii commented 8 years ago

I don't see how storing the number of the records would help us.

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.

You can still fetch all of the flag records for a script.

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 flags. e.g. If we detect we are on the route I said above then we can do the find with aModelListQuery.and({ flaggedCount: { $gt: 0 } }); instead.

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?

Martii commented 8 years ago

but only needed on the this route.

Don't forget to add the QSP of ?flagged=true. So for example... run the PR with the commented out line that I mentioned and view http://localhost:8080/users/MAX30/scripts?flagged=true with local pro and you'll see that user has a flag... but if you don't comment out that line you see nothing. If the person who flagged this user was just flag bombing say the first 10 scripts we would never know until it became critical and showed up with flags (which is a combination of up/down votes and actual flags)... just a side note that user that flagged his script is a random flagger that I've had to clear the flags everytime I recheck all flags manually e.g. there is no rhyme or reason to that users flagging.

Does this specific example make more sense?

Martii commented 8 years ago

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
}
Martii commented 8 years ago

a-ha it is already set with aOptions.isUserScriptListPage... so modified (and corrected) code would be:


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
}
Martii commented 8 years ago

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.

Martii commented 8 years ago

So summarizing:

  1. Close this issue as fixed when #643 is approved... e.g. ignore flag bombing until critical mass has been achieved and make it more difficult to determine if it is a flag bomber (possibly change text to "no critical flagged scripts." for clarity)
  2. With the flagged QSP show all scripts even if they aren't flagged on user script list pages (the most recent code snippet here) ... (this one is growing on me but deviates from only showing actual flagged scripts in the list but still needs #643... see next option point)
  3. Track the actual Flag's count in each model... a little more coding work but filters out scripts that aren't flagged... nicer display but can hide if a flag bomber is targetting larger upvoted scripts.
  4. Some other option not mentioned yet.

/end of this noise for a time to ponder

Martii commented 8 years ago

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.

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?

Martii commented 8 years ago

Hmmm perhaps...

4a) QSP of allflags=true, or equivalent naming (absolute, all, anyflags, anyflagged, absoluteflags, absoluteflagged, absflags, absflagged etc. e.g. under the threshold... EDIT: perhaps flaggedFilter=critical, flaggedFilter=absolute, and flaggedFilter=none instead of true or false for the QSP... or even modifying flagged to be multi-state "type" instead of true/false with flagged=critical, flagged=absolute and flagged=any), merged with Option 2 and Option 3 ... this would maintain flagged QSP human logic and allow for bombing checks via the URI e.g. this would be an inverse filter I think.

Martii commented 8 years ago

An example mockup for Option 4a in a list style/modeled after the Graveyard filter:

Martii commented 8 years ago

So @sizzlemctwizzle... have I convinced you yet?

@Zren wrote:

Virtual functions aren't serialized into the db, nor are their calculated values, ergo you can't query over them.

http://mongoosejs.com/docs/guide.html#virtuals

... which means in order to do this issue ticket I will most likely need to "serialize" (e.g. store) the absolute flag count into the database model schemas so I can query it to the appropriate list type (just need to center on a field name constant)... The Filters mockup wouldn't show for those without the privileges for the flagged QSP. Default would be critical flagged as it is currently now.

This is very doable and very helpful from my end of administration... and of course founders and root roles (e.g. higher ups).

Martii commented 8 years ago

One more selling point... if "pardoned" from #642 it would be nice to occasionally Admin+ check those that wouldn't normally show up e.g. we would need to be able to view the "Absolute Flags" for this.

sizzlemctwizzle commented 8 years ago

@Martii You've sold me on Option 4a.

Martii commented 8 years ago

@sizzlemctwizzle

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 flags.critical and flags.absolute ? We seem to be missing flags here too. (these tread on #262)

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. :)

sizzlemctwizzle commented 8 years ago

Yeah I'm fine with your proposed migration.

Martii commented 8 years ago

@sizzlemctwizzle Dev is converted... will upload momentarily the PR for quick review and conversion of production in ~40 minutes. Please do not run the PR on local production as it will hose it before the migration but dev is converted so it can be run against that. Thanks.

Martii commented 8 years ago

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.

Martii commented 8 years ago

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.

Martii commented 8 years ago

At everyone whois Admin+ ... I manually fixed one three Script records just to make sure the UI is working with Absolute Flags... so if you care to look you can filter on that... However please do not kill that flag because it's my unit test for all of this. Thanks. :)

Hopefully later this evening after food break I'll fix all the records of currently flagged items... then the list will grOW!

Martii commented 8 years ago

Done with DB migration into flags.absolute! Closing... check it out for Admins+.

@sizzlemctwizzle I would suggest a local copy of the DB backup now... and you could label it todays date with the current project version of v0.2.1 :) Thanks.

Martii commented 8 years ago

@sizzlemctwizzle Oh and probably the current commit HEAD hash (of 12b5b38) too... that way in case we need a specific integrated DB we have one at your disposal. TIA.


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.