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

Missing scripts within group #137

Closed
jerone opened this issue Jun 11, 2014 · 19 comments
Closed

Missing scripts within group #137

jerone opened this issue Jun 11, 2014 · 19 comments
Labels
bug You've guessed it... this means a bug is reported.

Comments

@jerone
Copy link
Contributor

jerone commented Jun 11, 2014

Some scripts are not showing up in groups when they should be.

For example the Github groups, it's size says 6, but it only shows 1. I know for sure that at least 5 scripts should be in that group.

@jerone jerone added the bug label Jun 11, 2014
@Zren
Copy link
Contributor

Zren commented Jun 11, 2014

Do you know that there were 5 other scripts before the UI update? Do you know the names of any of them? Or is the counter incorrect?

@jerone
Copy link
Contributor Author

jerone commented Jun 11, 2014

I know for sure that there were multiple before the new design. Most of those were mine.

--- Original Message ---

From: "Chris Holland" [email protected]
Sent: June 12, 2014 12:17 AM
To: "OpenUserJs/OpenUserJS.org" [email protected]
Cc: "Jeroen van Warmerdam" [email protected]
Subject: Re: [OpenUserJS.org] Missing scripts within group (#137)

Do you know that there were 5 other scripts before the UI update? Do you know the names of any of them? Or is the counter incorrect?


Reply to this email directly or view it on GitHub:
#137 (comment)

@jerone
Copy link
Contributor Author

jerone commented Jun 11, 2014

Github News Feed Filter for example still has the Github group when editing script info.

@Martii
Copy link
Member

Martii commented Jun 11, 2014

but it only shows 1

CONFIRM There was definitely more than one of his. Some, but not all, counts are off by at least 1... not entirely sure if that's due to account removal and/or script deletion.

  • 13 present/matched of mine in userscripts.org are present plus 2 additional in the last few days from other users.
  • 2 present/matched of mine in openuserjs.org plus 1 of yours for a test
  • 6 present/matched of mine in Unit Tests

Related:

@Zren
Copy link
Contributor

Zren commented Jun 11, 2014

Crap. It's probably got karma, giving it a negative flags value?

c80d85f

checks if script.flags == 0. It needs to be {flags: {$lt: 1}}

@Martii
Copy link
Member

Martii commented Jun 11, 2014

While we're in there should we sort/create via toLowerCase() ?? @sizzlemctwizzle?

@Zren
Copy link
Contributor

Zren commented Jun 11, 2014

A quick google search says we'll have to add a field into the data model to sort on it it seems.

@sizzlemctwizzle
Copy link
Member

Can't you just change the threshold values here:
https://github.com/OpenUserJs/OpenUserJS.org/blob/c80d85fb03e1c7bbf6069a20bb415551122f70a7/libs/flag.js#L4

There's only one script in production with a flag (I didn't remove it because it looks fine imo) so it shouldn't matter. You don't need to hardcode it imo.
On Jun 11, 2014 5:59 PM, "Chris Holland" [email protected] wrote:

Crap. It's probably got karma, giving it a negative flags value?

c80d85f
c80d85f

checks if script.flags == 0. It needs to be {flags: {$lt: 1}}


Reply to this email directly or view it on GitHub
#137 (comment)
.

@Martii
Copy link
Member

Martii commented Jun 11, 2014

one script in production with a flag

Two as of about an hour'ish ago... tried out the flagging feature on an obfuscated script.

@Zren
Copy link
Contributor

Zren commented Jun 12, 2014

Going to just remove the ability to filter out flagged scripts. They'll still show in the moderation page, but we need to get back check into flagging anyways for #134

@Zren
Copy link
Contributor

Zren commented Jun 12, 2014

@sizzlemctwizzle
Copy link
Member

has a script._groupId while the others do not

That's because the script._id's are stored on the Group model. script._groupId is just used to record what group a script created to enforce the "one group created per script limit".

@Zren
Copy link
Contributor

Zren commented Jun 14, 2014

Now that we can see group data in production... Greetings Rubber Ducky.

https://openuserjs.org/admin/json?model=Group&id=535844d78e5a7a54a7ebbca1

{
  "__v": 7,
  "_id": "535844d78e5a7a54a7ebbca1",
  "name": "Github",
  "rating": 0,
  "updated": "2014-06-14T17:35:15.548Z",
  "_scriptIds": [
    "5358446b8e5a7a54a7ebbca0",
    "531e4350d627516d13f98c3d",
    "532c284b21167c0000519cb9",
    "532c284b21167c0000519cba",
    "532c284b21167c0000519cbc",
    "532c284b21167c0000519cbb",
    "53916b28569129f76b46b075",
    "5392564e569129f76b46b095"
  ]
}

None of the scripts have a flagged property, Only some have the flags property. Most interesting though is the 404.

The two visible scripts are the first script, and the last script in that list (if order is kept).

Both of those have a _groupId but so does

https://openuserjs.org/admin/json?model=Script&id=532c284b21167c0000519cba

so that's not it.

Gods, this is annoying me. Think I'll try dumping these into in my dev env.

@Zren
Copy link
Contributor

Zren commented Jun 19, 2014

Finally wrote a script to import stuff from /admin/json.

Deleting the 404 scriptId did nothing. Turns out it was scriptListQuery.find({isLib: false}); needing to be scriptListQuery.find({isLib: {$ne: true}});.

PR incoming

@sizzlemctwizzle
Copy link
Member

needing to be scriptListQuery.find({isLib: {$ne: true}});

Yeah, I was confused why you didn't use that since it was used in the original code.

@Zren Zren closed this as completed in 842fd6c Jun 19, 2014
sizzlemctwizzle added a commit that referenced this issue Jun 19, 2014
Fix #137 : Missing scripts within group
@jerone
Copy link
Contributor Author

jerone commented Jun 19, 2014

This is better but still not completely fixed!

On the groups listing page it says size 8 scripts for Github, but when counting scripts on the Github group there are 6.
Other groups have the same problem.

@jerone jerone reopened this Jun 19, 2014
@sizzlemctwizzle
Copy link
Member

Could be the result of deleted scripts.
On Jun 19, 2014 10:12 AM, "Jeroen van Warmerdam" [email protected]
wrote:

This is better but still not completely fixed!

On the groups listing page https://openuserjs.org/groups it says size
8 scripts for Github, but when counting scripts on the Github group
https://openuserjs.org/group/Github there are 6.


Reply to this email directly or view it on GitHub
#137 (comment)
.

@Martii
Copy link
Member

Martii commented Jun 19, 2014

This is better but still not completely fixed!

Could be the result of deleted scripts.

Quite possibly... that page rings a bell now that it's showing the 6 that were in it... @jerone I think you made a comment about sizzles script being in the wrong group (yours) a while back and he replied... you have 5 and sizzles makes 6.... We have a total of 7 scripts available at this time for GH that I can see and I'm a contributor on the newest one and I know he (xthexder) didn't add it to the group yet (he hasn't updated too).

@sizzlemctwizzle
Is there a way to copy/link to the current pro DB to dev temporarily and we can just roll back to a previous OUJS checkout without exposing the current secrets and check for absolute sure?

@Zren
Copy link
Contributor

Zren commented Jun 19, 2014

1 of the 7 origional scripts was a 404 (deleted most likely). I made a test script under GitHub (bumping it to 8 scripts) . After closing this issue, I deleted that script (as you can't remove groups #108 ). The group counter not dropping is already filed under #93 so I'll close this.

@Zren Zren closed this as completed Jun 19, 2014
@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
bug You've guessed it... this means a bug is reported.
Development

No branches or pull requests

4 participants