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

Admin Feature: Show absolute flag count #641

Closed
sizzlemctwizzle opened this issue Jun 10, 2015 · 29 comments
Closed

Admin Feature: Show absolute flag count #641

sizzlemctwizzle opened this issue Jun 10, 2015 · 29 comments
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. DB Pertains inclusively to the Database operations. feature Something we don't already have implemented to the best of knowledge but would like to see. migration Use this to indicate that it may apply to an existing or announced migration. UI Pertains inclusively to the User Interface.

Comments

@sizzlemctwizzle
Copy link
Member

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.

@sizzlemctwizzle sizzlemctwizzle added enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. UI Pertains inclusively to the User Interface. CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. labels Jun 10, 2015
@sizzlemctwizzle sizzlemctwizzle added feature Something we don't already have implemented to the best of knowledge but would like to see. and removed enhancement Something we do have implemented already but needs improvement upon to the best of knowledge. labels Jun 10, 2015
@Martii
Copy link
Member

Martii commented Jun 10, 2015

arggh wrong issue sorry... moved.

@TimidScript
Copy link

@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
Copy link
Member

Martii commented Jul 17, 2015

@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 Martii self-assigned this Oct 14, 2015
@Martii
Copy link
Member

Martii commented Oct 14, 2015

@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
Copy link
Member

Martii commented Oct 14, 2015

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
Copy link
Member Author

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" [email protected] 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
#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
#641 (comment)
.

@sizzlemctwizzle
Copy link
Member Author

That looks good.
On Oct 13, 2015 10:01 PM, "Marti Martz" [email protected] wrote:

Btw here is a draft of what #643
#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
#641 (comment)
.

Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Oct 15, 2015
* 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
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Oct 17, 2015
* 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
@Martii Martii mentioned this issue Oct 17, 2015
@Martii
Copy link
Member

Martii commented Oct 17, 2015

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
Copy link
Member Author

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" [email protected] wrote:

Just a FYI we still probably need to store the absolute counts in the the
Script and User models to affect #643
#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
#641 (comment)
.

@Martii
Copy link
Member

Martii commented Oct 17, 2015

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
Copy link
Member

Martii commented Oct 17, 2015

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
Copy link
Member

Martii commented Oct 17, 2015

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
Copy link
Member

Martii commented Oct 17, 2015

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
Copy link
Member

Martii commented Oct 17, 2015

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
Copy link
Member

Martii commented Oct 17, 2015

So summarizing:

  1. Close this issue as fixed when Admin Feature: Show which users have flagged something #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 Admin Feature: Show which users have flagged something #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 pushed a commit to Martii/OpenUserJS.org that referenced this issue Oct 18, 2015
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Oct 18, 2015
* Denote that this is only searching for critical flags instead of all flags.

Applies to OpenUserJS#641
@Martii
Copy link
Member

Martii commented Oct 18, 2015

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
Copy link
Member

Martii commented Oct 18, 2015

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
Copy link
Member

Martii commented Oct 18, 2015

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

  • issue-641mockupoption4adefault opt
  • issue-641mockupoption4aabsolute opt

@Martii Martii added the DB Pertains inclusively to the Database operations. label Oct 20, 2015
@Martii
Copy link
Member

Martii commented Oct 20, 2015

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

@Martii Martii added the question A question has been encountered by anyone and has remained unanswered until cleared. label Oct 20, 2015
@sizzlemctwizzle
Copy link
Member Author

Yeah I'm fine with your proposed migration.

@Martii Martii added migration Use this to indicate that it may apply to an existing or announced migration. and removed question A question has been encountered by anyone and has remained unanswered until cleared. labels Oct 20, 2015
@Martii
Copy link
Member

Martii commented Oct 21, 2015

@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 pushed a commit to Martii/OpenUserJS.org that referenced this issue Oct 21, 2015
* 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
@Martii
Copy link
Member

Martii commented Oct 21, 2015

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
Copy link
Member

Martii commented Oct 21, 2015

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 pushed a commit to Martii/OpenUserJS.org that referenced this issue Oct 21, 2015
* 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
@Martii
Copy link
Member

Martii commented Oct 21, 2015

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
Copy link
Member

Martii commented Oct 22, 2015

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 Martii closed this as completed Oct 22, 2015
@Martii Martii removed their assignment Oct 22, 2015
@Martii
Copy link
Member

Martii commented Oct 22, 2015

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

Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Dec 25, 2015
* 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
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Mar 26, 2016
* 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
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Mar 26, 2016
* This time positive increment when unvoting

Post fix for OpenUserJS#641
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Oct 23, 2020
* 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
Martii added a commit that referenced this issue Oct 23, 2020
* 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 #1109

Applies to #126 and post #641

Auto-merge
@OpenUserJS OpenUserJS locked as resolved and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. DB Pertains inclusively to the Database operations. feature Something we don't already have implemented to the best of knowledge but would like to see. migration Use this to indicate that it may apply to an existing or announced migration. UI Pertains inclusively to the User Interface.
Development

No branches or pull requests

3 participants