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

Better way to detect employees using less bandwidth/fewer API requests #341

Closed
GaurangTandon opened this issue Jul 4, 2018 · 21 comments
Closed

Comments

@GaurangTandon
Copy link

Installed Version: 2.2.3DEV Environment: Tampermonkey

Current Behaviour

image

Steps to reproduce

Visit this page - https://meta.stackexchange.com/questions/312038/wed-like-your-feedback-on-our-new-code-of-conduct and see comments by profile Donna

Note: When I run the API on Donna's profile, the is_employee property is set to true. So, I am not at all sure what the issue actually is :/ I reviewed the code under markEmployees, but it looks okay and there shouldn't honestly be an issue. And yet there is...


Features Enabled

["Appearance-colorAnswerer","Appearance-isQuestionHot","Appearance-markEmployees","Appearance-metaChatBlogStackExchangeButton","Appearance-metaNewQuestionAlert","Appearance-tabularReviewerStats","Appearance-addTimelineAndRevisionLinks","Comments-autoShowCommentImages","Comments-confirmNavigateAway","Editing-addSBSBtn","Editing-inlineEditorEverywhere","Flags-flagPercentages","Flags-flagPercentageBar","Sidebar-hideHireMe","Sidebar-hideChatSidebar","Sidebar-hideLoveThisSite","Sidebar-hideHotNetworkQuestions","Chat-replyToOwnChatMessages","Extras-showMetaReviewCount","Extras-copyCode","Extras-dailyReviewBar","Extras-showQuestionStateInSuggestedEditReviewQueue"]
@shu8 shu8 self-assigned this Jul 4, 2018
@shu8 shu8 added this to the 2.3.0 milestone Jul 4, 2018
@shu8
Copy link
Member

shu8 commented Jul 4, 2018

@GaurangTandon Reproduced; I'll look into it, thanks!

shu8 added a commit that referenced this issue Jul 4, 2018
@shu8
Copy link
Member

shu8 commented Jul 4, 2018

@GaurangTandon temporarily fixed in dev 2.2.11.

It was because the API only returns 30 items by default but that page had 59 unique users. I've changed the size to 100 now, but the function could probably do with making sure if the number of IDs is greater than 100 then splitting them into multiple requests, but I'll do that another time!

@shu8 shu8 removed the in progress label Jul 4, 2018
@GaurangTandon
Copy link
Author

A few remarks:

  1. Please use filters! You are only using the is_employee and user_id properties under the API call. You need a single apple but you're fetching the whole fruit shop! I'll submit a pull request within a few hours to use filters, not just here but in all other API calls.

  2. Could you cache the IDs with GM_setValue? Currently the API fetches data associated with these IDs every time I load any SE webpage. While I am on vacations, I open SE webpages at least hundred times a day (not exaggerating). You could rather cache the user_ids of employees and update this list once everyday. The cache would be small as there are relatively fewer employees.

  3. You can easily shorten the number of IDs you pass to by checking if a user has the diamond next to their name. All employees have a diamond next to their name on each site they participate. (for example, Donna had the diamonds on MSE and SO, but not GD.SE where they didn't participate)

I'll submit a PR asap.

@shu8
Copy link
Member

shu8 commented Jul 4, 2018

@GaurangTandon You're right about the filters, I should have used them from the beginning :/

We could possibly cache them, the only problem I can see is that they each have a different user ID on different sites, but I guess you'd only really be seeing them on a few main ones?

About the diamonds, some employees don't necessarily have diamonds, so I don't think we should do that -- see e.g. https://meta.stackexchange.com/questions/216414/are-there-moderators-without-a-diamond#comment700653_216416, or https://islam.meta.stackexchange.com/a/1003.

GaurangTandon added a commit to GaurangTandon/sox that referenced this issue Jul 4, 2018
1. remove redundant `unique` method by directly checking inserted ids
2. use the mod flair as a hinge for reducing number of checked ids (see soscripted#341 (comment))
3. also used filters in SE API
4. fixed position of SO logo to be on the left of the diamond in all cases (in curr version, it's on the right inside .comment and on the left for .user-details)
5. consistency improvements
@GaurangTandon
Copy link
Author

GaurangTandon commented Jul 4, 2018

About the diamonds, some employees don't necessarily have diamonds

...well, they don't have their is_employee set to true either. Here's Laura for example. But I would rather that Laura has retired since then. Her last activity on Meta.SE was in 2017. Let me ask Taverners if there are currently working devs without a diamond...

Till then keep the PR on pause.

@j-f1
Copy link
Contributor

j-f1 commented Jul 4, 2018

@GaurangTandon Don’t forget that there are employees who aren’t devs. SO also employs salespeople, designers, community managers, data scientists, and probably other people too.

@GaurangTandon
Copy link
Author

GaurangTandon commented Jul 4, 2018

@j-f1 That's right, being a dev myself I was biased towards devs :P

@shu8

some employees don't necessarily have diamonds,

If we query users/moderators, we can get the list of employees on that site. We can run this query once a week and cache the results.

We could possibly cache them, the only problem I can see is that they each have a different user ID on different sites, but I guess you'd only really be seeing them on a few main ones?

Yah, right, we should use localStorage for that as it would give us SE site specific results.

I'll do a PR tomorrow it's super late here!

@GaurangTandon
Copy link
Author

@shu8 The script is ready. You can read the full details on the relevant StackApps question page. Now, the thing here is that, only 75% of the work is complete. The script can fetch the user_ids for us. Brock mentioned that we need to setup a chron job to automate that task weekly, and store its results in a public JSON file. Once that is done, a public JSON file of the format [userID1, userID2, userID3, ...] would be ready.

I instead think it'd be better to run this task from the client weekly, given that SOX has its own access token already, and store the associated accounts_ids into the tempermonkey storage. Then, I can write code that would utilize this cached file, requesting more user_ids from the account_ids for individual SE sites as necessary.

I'll work this out in a day or two.

@shu8
Copy link
Member

shu8 commented Jul 6, 2018

@GaurangTandon nice! that google apps script is pretty neat!

@GaurangTandon
Copy link
Author

GaurangTandon commented Jul 7, 2018

@shu8 I've hit an obstacle. Namely, we cannot run the weekly checks looking for userIDs from the client side, because a) the API method takes more than twenty minutes to execute. That's way too long ttwenty minutes for every person who installs SOX. b) scraping the Google Apps Script page requires the client to set Access-Control-Allow-Origin: *, using extensions like this one. We can't expect users to have such extensions installed, nor can we force them to do so.

The only solution left to us, seems to be:

  1. setup a public Github file and make the client GET it weekly via the GitHub API.
  2. setup the script to run weekly updates on the public GitHub file via a cron job - from the server side

Now, when I say "server side", I mean a PC/mac that belongs to one of SOX core contributors. So, are you willing to run this job on your PC @shu8?

Again, this may seem like an "overkill" (specially since the current method still works), but I think that if the "overkill" favors extremely reduced bandwidth consumption, we should certainly do it. While I certainly love the employee marker, I dislike shelling out so much bandwidth for it every time I load the SE page :(

Thoughts?

@shu8
Copy link
Member

shu8 commented Jul 7, 2018

@GaurangTandon sorry, I don't think I'd be able to have it on my computer, it's often shared and in a few months I'll be at uni!

I appreciate you want to save bandwidth, so how about we create a local store of employee IDs over time? So we can keep it like you've currently changed it to, and then with every employee, add them to a local GM Value/localStorage variable and don't add the id to the ids array if the ID already exists in the saved array. Although it will still use bandwidth, it will be much less over time, especially if you frequent the same few Meta sites. What do you think?

@GaurangTandon
Copy link
Author

it's often shared and in a few months I'll be at uni!

@shu8 Ah, same here. Except that I'm leaving for uni this month :O

Anyway, I don't exactly understand what you're trying to propose here. What do you mean by a "a local store of employee IDs over time?"

@shu8
Copy link
Member

shu8 commented Jul 7, 2018

@GaurangTandon I was thinking of storing employee user IDs as they are found (using the current method of sending all user IDs to the API) and so we wouldn't need to fetch for known employee IDs in the future for a given client

However, now that I think about it, it probably won't save many requests because there are more non employees than employees 😕

@GaurangTandon
Copy link
Author

GaurangTandon commented Jul 10, 2018

I guess we're stuck at the existing method then? I'll admit it's still very expensive.

Well, then, what else can be done :(

@shu8
Copy link
Member

shu8 commented Jul 10, 2018

@GaurangTandon hmm, I just thought about that YQL thing you mentioned -- we could use that to access the google apps script?

@GaurangTandon
Copy link
Author

I guess that would work...I've never used YQL though. Wanna give it a shot? See if you can scrape the GA scripts page without requiring to disabling CORS.

@shu8
Copy link
Member

shu8 commented Jul 10, 2018 via email

@j-f1
Copy link
Contributor

j-f1 commented Jul 10, 2018

There are also things like crossorigin.me which will allow you to fetch resources from any URL (the only caveat being that they don’t get session cookies).

@shu8
Copy link
Member

shu8 commented Jul 10, 2018

@j-f1 Cool, I never knew that existed! Thanks, I'll look into it!

@shu8
Copy link
Member

shu8 commented Jul 14, 2018

I couldn't get it to work for yahoo, I think they've stopped supporting extracting the HTML because I get an error that 'html table is not supported'.

crossorigin.me didn't work either, and I think that may be because it has a 2MB limit? But the error I get is a 522 no-referrer-when-downgrade :/ (instead of a 413 like the website says)

I found https://cors-anywhere.herokuapp.com/ and it works with that: https://jsfiddle.net/shub01/ohd86uby/2/

I'm going to go back to the features that use yahoo's YQL and see if they still work!

@shu8 shu8 removed the in progress label Jul 16, 2018
@shu8 shu8 changed the title The employee marker doesn't always work Better way to detect employees using less bandwidth/fewer API requests Jul 16, 2018
@shu8 shu8 removed this from the 2.3.0 milestone Aug 11, 2018
shu8 added a commit that referenced this issue Sep 2, 2018
* new dev

* #305

* #307

* #310

* #310

* Better fix for #310

The old fix placed the SOX button to the right of the username, but the username only shows up in the top bar if you're logged in. This meant the SOX button would be absent if you were logged out. This new code should fix that.

* #310

* #309, #313

* #311, #312

* #256

* #319, #317, #316

* #256

* #318

* #294 deprecate enhanced editor

* #308, other small fixes

* #320

* minor fixes (#324)

change scroll top top feature JSON info

* fixes #329 (#330)

also reduces jQuery dependency

* v2.1.4 DEV before push to master

also adds #326

* v2.2.0

* inital DEV push

* update readme to state GM not supported; #306

* reduce dependency on `hotkeys`+minor details (#333)

* #334 fixed

* #334

* #334

* #338

* #335, #338

* Micro optimizations (#336)

* update source link

* micro optimizations

- converted jQuery .not to CSS selector for faster perf (https://stackoverflow.com/questions/8845811/performance-differences-between-using-not-and-not-selectors)
- introduced a separate function to eliminate thrice-repeated (!) code, to keep it DRY

* fixes some issues (#339)

described in #338 (comment)

* fixes #338

* fixes #338 (#340)

1. used addEventListener instead of onload
2. descriptive names (`reader`)
3. optimized extraction of the image URL from the `data`
4. made all declarations `var` and moved them into one place for consistency
5. shortened the POST url to simply `'/upload/image?https=true',` ;P

* #322 implemented

* Sticky property

* Update sox.user.js

* Updating button colors

The main SE buttons use a lighter color than the SOX buttons

* Update sox.css

* Minor fixes

Patched a problem with vote buttons, and fixed the color of the notify on edit button

* Update sox.user.js

* minor changes/fixes

* #341 temporary fix

* Updated mod diamond icon

Updated to the icon, to match the one used by the new topbar

* Updated diamond icon

* Updated mod diamond

* improvements in colorAnswerer (#344)

* improvements in colorAnswerer
1. constant variables+cache answeredID
2. reduce jQuery dependency+shorter a[href] selector via user ID
3. change coloration

* optimization: use filters throughout (#345)

reduces bandwidth consumption

* fixes a few features; adds CSS

Fixes moveBounty, dragBounty, copyCommentsLink.
Adds class for colorAnswerer.
Updates jQuery

* fixes quickAuthorInfo fontawesome icon

* fix a few of #308

* Major improvements in code extensibility, reduced API usage, eliminate redundancy, etc. (#348)

* fixes various bugs mentioned in #348

* fixes standOutDupeCloseMigrated for search pages

* remove unwanted console logs

* #347, various minor tweaks

* fixes small bug in parseCrossSiteLinks

e.g. on this comment: https://meta.stackoverflow.com/questions/370902/i-need-a-lot-of-help-to-write-a-query-for-the-stack-exchange-data-explorer-sh?cb=1#comment608548_370902

* Fixed Meta SE's chat link

Added a / to the end of Meta SE's if statement to fix it. Also replaced "discuss" with "meta" for Area 51 (to match the new URL for Area 51 Discussions), shortened a bit of code, and added some comments

* Fixed Meta SE links

* Changed topbar z-index

Changed the fixed topbar's z-index to match the one on SO, in order to prevent elements like the usercard (z-index: 1000) from covering it

* Made it clearer what the checkbox does

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Fixed Area 51 Discussions bug

* #348 minor tweak

* fix (#350)

...for answerer ID 13 also matching commenter ids beginning with 13 (1345, 13555, etc.)

* #352, #353 remove yahoo YQL use

* #352

* PR/351 (#355)

* update getQuestionTags

* provide immediate response to user

* minor changes

* #325: remove rangyInputs dependency (#354)

* bump version number

* remove downvotedPostsEditAlert; remove rangyinputs `require`; implement mini changelog in dialog

* install stale and no-response bots

* #356 added, minor bug fix

* start #349, start to move to es6

* onlyShowCommentActionsOnHover bug fix

* #357

* deprecate fixedTopbar feature for #349

* add showTagWikiLinkOnTagPopup feature

* #359 fixed

* #358 fixed

* fixes #360

* fixes for #361

* #361 changes

* v2.3.0 final
shu8 added a commit that referenced this issue Feb 3, 2019
* new dev

* #305

* #307

* #310

* #310

* Better fix for #310

The old fix placed the SOX button to the right of the username, but the username only shows up in the top bar if you're logged in. This meant the SOX button would be absent if you were logged out. This new code should fix that.

* #310

* #309, #313

* #311, #312

* #256

* #319, #317, #316

* #256

* #318

* #294 deprecate enhanced editor

* #308, other small fixes

* #320

* minor fixes (#324)

change scroll top top feature JSON info

* fixes #329 (#330)

also reduces jQuery dependency

* v2.1.4 DEV before push to master

also adds #326

* v2.2.0

* inital DEV push

* update readme to state GM not supported; #306

* reduce dependency on `hotkeys`+minor details (#333)

* #334 fixed

* #334

* #334

* #338

* #335, #338

* Micro optimizations (#336)

* update source link

* micro optimizations

- converted jQuery .not to CSS selector for faster perf (https://stackoverflow.com/questions/8845811/performance-differences-between-using-not-and-not-selectors)
- introduced a separate function to eliminate thrice-repeated (!) code, to keep it DRY

* fixes some issues (#339)

described in #338 (comment)

* fixes #338

* fixes #338 (#340)

1. used addEventListener instead of onload
2. descriptive names (`reader`)
3. optimized extraction of the image URL from the `data`
4. made all declarations `var` and moved them into one place for consistency
5. shortened the POST url to simply `'/upload/image?https=true',` ;P

* #322 implemented

* Sticky property

* Update sox.user.js

* Updating button colors

The main SE buttons use a lighter color than the SOX buttons

* Update sox.css

* Minor fixes

Patched a problem with vote buttons, and fixed the color of the notify on edit button

* Update sox.user.js

* minor changes/fixes

* #341 temporary fix

* Updated mod diamond icon

Updated to the icon, to match the one used by the new topbar

* Updated diamond icon

* Updated mod diamond

* improvements in colorAnswerer (#344)

* improvements in colorAnswerer
1. constant variables+cache answeredID
2. reduce jQuery dependency+shorter a[href] selector via user ID
3. change coloration

* optimization: use filters throughout (#345)

reduces bandwidth consumption

* fixes a few features; adds CSS

Fixes moveBounty, dragBounty, copyCommentsLink.
Adds class for colorAnswerer.
Updates jQuery

* fixes quickAuthorInfo fontawesome icon

* fix a few of #308

* Major improvements in code extensibility, reduced API usage, eliminate redundancy, etc. (#348)

* fixes various bugs mentioned in #348

* fixes standOutDupeCloseMigrated for search pages

* remove unwanted console logs

* #347, various minor tweaks

* fixes small bug in parseCrossSiteLinks

e.g. on this comment: https://meta.stackoverflow.com/questions/370902/i-need-a-lot-of-help-to-write-a-query-for-the-stack-exchange-data-explorer-sh?cb=1#comment608548_370902

* Fixed Meta SE's chat link

Added a / to the end of Meta SE's if statement to fix it. Also replaced "discuss" with "meta" for Area 51 (to match the new URL for Area 51 Discussions), shortened a bit of code, and added some comments

* Fixed Meta SE links

* Changed topbar z-index

Changed the fixed topbar's z-index to match the one on SO, in order to prevent elements like the usercard (z-index: 1000) from covering it

* Made it clearer what the checkbox does

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Many fixes to dialog buttons

Fixes many of the bugs from #308

* Fixed Area 51 Discussions bug

* #348 minor tweak

* fix (#350)

...for answerer ID 13 also matching commenter ids beginning with 13 (1345, 13555, etc.)

* #352, #353 remove yahoo YQL use

* #352

* PR/351 (#355)

* update getQuestionTags

* provide immediate response to user

* minor changes

* #325: remove rangyInputs dependency (#354)

* bump version number

* remove downvotedPostsEditAlert; remove rangyinputs `require`; implement mini changelog in dialog

* install stale and no-response bots

* #356 added, minor bug fix

* start #349, start to move to es6

* onlyShowCommentActionsOnHover bug fix

* #357

* deprecate fixedTopbar feature for #349

* add showTagWikiLinkOnTagPopup feature

* #359 fixed

* #358 fixed

* fixes #360

* fixes for #361

* #361 changes

* v2.3.0 final

* 2.3.0 DEV

* fix for #363

* implemented 'feature packs' in settings dialog

* debugging for #363, fixes #364

* #363 debugging

* fix for #363

* #361 implemented (access token no longer mandatory); minor code cleanup

* #365

* fix #368

* Add #315

* fix #366

* start to use linting

* fix #372

* add #370

* clean up code

* implement #374

* fix #373

* add #370; deprecate pasteImagesDirectly

* #374 replace 'help' instead of prepend new link

* change indentation to 2 spaces

* lint

* implement #296

* implements #376 (linkify meta diamond dialog title)

* fix #375 (topAnswers post score not being found)

* improve customMagicLinks feature -- add button to help menu on all pages

* fix sticky vote buttons feature

* keep new meta posts under diamond even after clicking (#378)

* editComment improve design, add more defaults (#377)

* fix better css feature

* editComment: make dialog delete button smaller

* improve editComment for #377; allow editing; new setting storage format

* add delay to comment features for links to deleted comments #379

* bump to v2.4.0

* Remove accidental letter introduced into file

* Bump installation link version numbers

* Lint
@shu8
Copy link
Member

shu8 commented Jun 2, 2019

Closing this in favour of the API caching that was implemented as part of #391; this will reduce the number of API requests made after initially performing them as they will be locally cached for a short period of time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants