Include rating in meta JSON

[DenverCoder1]

Rating is missing from meta JSON

Example: https://openuserjs.org/meta/NatoBoram/YouTube_Comment_Blacklist.meta.json

installs and issues appear but rating does not.

This would be useful for https://github.com/badges/shields/issues/5505.

[Martii]

Rating is missing from meta JSON

Why do you think that it is missing?

appear but rating does not.

Why do you think that should be available? i.e. give an concrete example of why it should be added.

This would be useful for https://github.com/badges/shields/issues/5505.

See previous question.

[DenverCoder1]

From the fact that shields.io support many rating badges, it seems like being able to display the rating of a script in a readme is popular among users. Therefore it would be convenient to have an API to retrieve that value.

As it's a statistic that appears on the site, it would make sense if it were in the API, although you can feel free to disagree.

[Martii]

I think https://github.com/badges/shields/pull/8080#discussion_r896323875 in https://github.com/badges/shields/pull/8080 is rather important as well as GF has splits and ours I know is an algorithm that was hand verified... which is quite complex and not just a simple 0 to 5 star rating system.

I'll think about it and add my 2 cents in later but you have to sell it on me harder at this time. It is simple enough to do but logically and logistically how it will affect retrieval is always a concern. We aim to minimize (traffic too). Obviously others can pipe in Cc: @NatoBoram for example.

[DenverCoder1]

I see what you mean. I'm not too familiar with how the rating system works and wasn't sure if it was a simple count of votes or something more complex.

If the number of votes is something that's easily accessible, maybe that would be more meaningful as a badge statistic?

[Martii]

Both are currently static values but both are calculated on-the-fly (to an aggregation using your terms elsewhere)... so can be positive or negative. Other factors also affect both calculated values such as flagging and moderation.

[Martii]

calebcartwright wrote for GF PR:

I think it would be best to at least pull out the rating badges from this PR so we can go ahead and get the rest of them merged, and then circle back to if/when/how on the ratings badges

We can add ratings in but it would be nice to know if there the negative/positive equivalent/inverse of nonNegativeInteger. As I don't use joi in the documentation it doesn't come up with a quick document search in their current API.

I should also give fair warning that our models store rating as a Number which could possibly include floating point. I've never actually seen one but a lot of it is buried behind status bars. I seem to recall it's integer math but don't quote me just yet.

[DenverCoder1]

would be nice to know if there the negative/positive equivalent/inverse of nonNegativeInteger

nonNegativeInteger is actually from shields.io's validator.js where effectively:

nonNegativeInteger is defined as Joi.number().integer().min(0).required() anyInteger is defined as Joi.number().integer().required() I suppose the same could be done for negative only by using max()

https://github.com/DenverCoderOne/shields/blob/5e6d68123a171c9d1deb73f01ea3d1396897e1ee/services/validators.js#L11-L14

[Martii]

There you go... confirmed it's positive/zero/negative integers (i.e. integer math at this time).

[DenverCoder1]

Awesome, thanks :+1:

As per Caleb's comment (https://github.com/badges/shields/pull/8080#discussion_r897278144) about how Greasyfork's rating badges should be considered for a later time, I'm thinking this similarly could be made a separate PR from #8081 because it similarly is a different kind of rating system than the ones they usually support.

In the meantime, this should allow people to make it with custom dynamic badges, so that's great.

Thanks for all your help with this.

[Martii]

@DenverCoder1

Thank you for all your help too... btw you'll probably be needing these for the normal coloring at some point:

https://github.com/OpenUserJS/OpenUserJS.org/blob/a9cbf7d6670d5232bae44f284129a63d758210fb/public/less/bootstrap/oujs-variables.less#L469-L483

... the state-whatever-bg is our normal coloring for backgrounds. Please ignore the comments as that is for a bootstrap migration (and #1903 for some error foregrounds matching in the palette) which is WAY down the road or if you want to show off some color samples I can apply them to dev and see how it looks all around and perhaps merge them in as a default system wide.

I am thinking...:

• when zero use info for whatever indicating nominal
• when positive use success for whatever indicating a positive trend
• when negative use warning for whatever indicating a negative trend

If you feel it is doesn't have enough contrast and not readable text the current corresponding reference for the foreground is at:

https://github.com/OpenUserJS/OpenUserJS.org/blob/a9cbf7d6670d5232bae44f284129a63d758210fb/public/less/bootstrap/oujs-variables.less#L17-L19

I've temporarily unlocked this issue in case you want to add anything (text or emoji). Please note it will relock within 24 hours of silence by our automation since it is closed. We're also displaying other solidarity colors atm but that will eventually revert to these code point references.

[DenverCoder1]

I think typically (based on the examples on their site), shields.io likes to stick to their own color palette by default for consistency.

They do allow users to manually override the colors themselves, though, to match whatever theme they'd like.

Edit: I do notice a few that don't follow this, so possibly they will allow it. I suppose it would be up to whoever reviews to decide.

[Martii]

It's up to you but probably at least yellow/blue or informational/ and one of the greens (perhaps plain green in their palette; bootstrap doesn't have a softer green category state... but whatever you think) for negative/zero/positive would be consistent with our Install button values currently used.

I use warning on some items and danger on extreme cases... very rarely do I use green because our install button doesn't currently do that like uso - Count Issues did back in the USO days... just sort of a laid back approach and let the user decide based off a common pattern.

Btw I'm grateful you changed downloads to installs recently on that PR. :) It wasn't a deal breaker but it is our terminology to use Installs. :smile_cat:

[Martii]

@DenverCoder1

Sorry you had to be an innocent bystander from that maintainers poor attitude today. Please note he is blocked on GH and if continues will be blocked further out in the network. Please feel free to open a generic team discussion either on OUJS or GH repo here if you need additional questions answered as I definitely won't be answering any kind of summons over there. We purposely have learned that a single point of presence is a good approach to weed out the questionable project maintainers.

If you get some time it would be highly appreciated if you could check their DoS protection e.g. do they really read headers and such as I'd rather not have our automation automatically ban them in case they are ignoring the well known headers. As I mentioned over there and apparently they want to be treated as a Guest from what I gather and turned down the option of a reciprocal API end-point.

As far as caleb wrote:

The way this, and the other collection schemas are currently structured will allow for empty arrays. If this is a valid scenario we could encounter in the response then we'll need to handle that scenario in the respective children classes that are trying to grab these values (array.at will return undefined which would then lead to an error after attempting member access).

If this is in reference to our JSON API there should never be an empty array. He's not being real clear from my perspective but maybe this info will help you since I'm still fuzzy on some of their code base.

Which noun do you think will feel more natural to OpenUserJS users, installs or downloads?

Mentioned previously :smile_cat: but of course you can use your best judgement. It's clear he didn't read all references to us now. *le sigh*

Can you share some context on what an issue represents on this platform? Is it more analogous to a bug tracker/problem reporting/etc. as opposed to an issue in tools like GitHub or Jira that can represent 'non-negative' things, like a story or task?

Can be anything including non-negative... could be praise too. Usually those are closed right away if the discussion owner does that. Could be just a TODO list by the Author as well.

Similar comment here as one I mentioned on another of your PRs, but this transform function has enough going on that I think unit tests would be helpful for us to have going forward.

Cited back on an edit where the reverse is required. Re-cited linkage for clarity here.

Leaving reopened unlocked again for same automation task in case you care to comment or emoji. Again if you need to open a team discussion you are quite welcome to instead.

[Martii]

@DenverCoder1

As a "bad news" FYI...

Re:

if you could check their DoS protection e.g. do they really read headers and such as I'd rather not have our automation automatically ban them in case they are ignoring the well known headers.

We have found and confirmed strong, and repetitive, evidence in our logging that Shields is not reading and obeying the headers at all at the very least with dynamic badges. A DoS routine was implemented a few days ago as an additional test and it has been found that all three of Shields servers are promoting a potential DDoS attack vector by triggering and documenting the issue. While the ranges are not critical at this moment, due to the lack of interest by most Authors, it could be detrimental to any site since Shields isn't paying attention to them. We have had the complete dataset cross verified by another party.

The following time difference (HMS) is submitted for a snapshot snippet of 2020-07-08 for multiple requests to the same dataset object on a single server:

0:01:06         200
0:28:49         200
3:00:14         200

... as I mentioned over there we do have limits and they are clearly being ignored. It isn't even consistent from what we can tell. Multiplied by the current, known, three servers they utilize this is where their error becomes highly distributed.

Because this is a security issue we are unable to divulge additional specifics in public and are pursuing potential further action if needed.

This also explains a bit why NPM has been rate limiting Shields as well (the mentioned 98% (ish btw) instead of calebs implied 100%)

and we're very, very familiar with GitHub's rate limits and how to work with them at mass scale.

Apparently this isn't true across the board which is what our initial supposition was and is now confirmed that at "mass scale" this could be prove to be a huge issue.

[DenverCoder1]

I'm not sure there's much I can do about that.

They do have details about reporting security issues at https://github.com/badges/shields/blob/master/SECURITY.md including an email address.

I get that there was an argument earlier, but I find it would be more effective to try ending the argument rather than continuing it as it seems they're the only ones that can help with the issue. You can always thank people for their opinion even if you don't agree.

[Martii]

I'm not sure there's much I can do about that.

Appreciate that you've looked into it.

it would be more effective to try ending the argument

We already did at the minimized comment. Depending on what action is needed here we'll probably need to rate limit them and revisit at a later date once they get their issues fixed. DDoS vectors are to be treated seriously and are outside any one opinion.

Thanks for looking into it again. :smile:

---

Removed Unit Tests on production due to premature closure of across-the-pond inconsistency on DDoS protection availability.