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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore lock icons on private orgs #2211

Merged
merged 4 commits into from Jul 10, 2019

Conversation

Projects
None yet
3 participants
@doms
Copy link
Contributor

commented Jul 7, 2019

What

Fixes #2210

Notes

After some digging, it seems as though the response from GitHub's API changed a bit(?), and some of the keys in the response aren't iterable. 馃

This just uses an explicit Object.values around the publicOrgs response object. 馃槃

Before

Screen Shot 2019-07-07 at 3 39 56 PM

After

Screen Shot 2019-07-07 at 3 40 08 PM

@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2019

They should be iterable; if this happens there must be a problem in the API or in the cache module. Can you show me exactly what publicOrgs looks like at first?

It seems to work for me in Chrome and Firefox

@doms

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

Perhaps 鈥渘ot iterable鈥 was a poor choice of words, but I wasn鈥檛 sure other words to use to describe it when I was debugging.

When I was console.log鈥檌ng the response of fetching the public orgs, the response would look like as shown:

Screen Shot 2019-07-08 at 10 21 10 PM

And so I figured I would iterate through the keys, and it seemed that sometimes I was getting 0 as a key, so I figured that鈥檚 why it was error鈥檌ng out with the publicOrgs.map is not a function 馃

let publicOrgs = await api.v3(`users/${getUsername()}/orgs`);
	
// console.log's to see the contents of `publicOrgs` response
console.log(publicOrgs)	
for (let key in publicOrgs) {
	console.log('key: ', key)
}
	
publicOrgs = Object.values(publicOrgs).map((orgData: AnyObject) => `/${orgData.login}`);

Screen Shot 2019-07-08 at 10 22 53 PM

As you see in the screenshot, we are getting the keys of:

  • 0 (the actual response as shown in the first image above)
  • status
  • ok

It seems that status: 200 and ok: true are key-value pairs, and the 0 key has the user orgs as the values 馃

So I figured I would just use Object.values(publicOrgs) to make sure I was getting a type that did have the map functionality (Array), and go from there. It seems to have solved the issue, but the weird part is that the original code works too in a different environment (https://repl.it/repls/DefiantCraftyInterface)

I'm getting similar behavior in both Brave (Chromium) and Firefox.

So yea, not entirely sure what鈥檚 happening, but I figured I would attempt to fix it maybe get some feedback in the code review on why mark-private-orgs might be broken on my end. 馃槃

@jerone

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

I can confirm this issue:
image

It is because the API itself returns an array, which is because of recent change now spread to an object in #2185. Resulting in using the map function on an object instead of the previous array.

@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jul 9, 2019

@jerone that鈥檚 funny 馃槄 good catch

The solution is to use return Object.assign(array, {.......})

@jerone

jerone approved these changes Jul 9, 2019

@doms doms force-pushed the doms:doms/fix-private-orgs branch from 67e8870 to cccae84 Jul 9, 2019

@bfred-it bfred-it added the bug label Jul 10, 2019

@bfred-it bfred-it merged commit 40b12d3 into sindresorhus:master Jul 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@doms doms deleted the doms:doms/fix-private-orgs branch Jul 10, 2019

notlmn added a commit to notlmn/refined-github that referenced this pull request Jul 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.