Skip to content

Hotfix instagram api #6922

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

Merged
merged 3 commits into from
Oct 1, 2020

Conversation

trimination
Copy link
Contributor

#6728

change from old to new instagram API

@codecov
Copy link

codecov bot commented Oct 1, 2020

Codecov Report

Merging #6922 into master will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6922      +/-   ##
==========================================
- Coverage   93.85%   93.80%   -0.05%     
==========================================
  Files         169      169              
  Lines       12220    12220              
==========================================
- Hits        11469    11463       -6     
- Misses        751      757       +6     
Impacted Files Coverage Δ
src/Adapters/Auth/instagram.js 100.00% <100.00%> (ø)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.92% <0.00%> (-0.67%) ⬇️
src/RestWrite.js 93.82% <0.00%> (-0.33%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.02% <0.00%> (-0.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 922dc76...78a68ac. Read the comment docs.

@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

thanks @timination can you please run npm test and fix/update tests for instagram auth 😃
We cannot merge until all tests are ok 💯

@trimination
Copy link
Contributor Author

trimination commented Oct 1, 2020

Sure, apologies I completely forgot about the test cases, they're updated now. Edit: standby, looking into this Travis CI error

@trimination
Copy link
Contributor Author

trimination commented Oct 1, 2020

No, just bug hunting on GitHub to make better use of my time. It was in response to #6728

@trimination trimination closed this Oct 1, 2020
@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

So LGTM based on the docs here: https://developers.facebook.com/docs/instagram-basic-display-api/guides/getting-profiles-and-media

Did you test the URL on a local implementation ?

@trimination trimination deleted the hotfix-instagram-api branch October 1, 2020 16:35
@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

@timination your fix seems good, my bad about the instagram docs, it was not clear. Based on https://developers.facebook.com/docs/instagram-basic-display-api/guides/getting-profiles-and-media your fixe is right !

@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

@timination can you push again your branch ?

@trimination trimination restored the hotfix-instagram-api branch October 1, 2020 16:38
@trimination
Copy link
Contributor Author

@Moumouls Should be back now

@trimination trimination reopened this Oct 1, 2020
@trimination
Copy link
Contributor Author

oh dear, I hit re-open and travis CI is gonna fail again lol. I'm not sure why, though. I get many F's even from master branch when running npm run coverage. Strange.

Copy link
Member

@Moumouls Moumouls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @timination for your contribution :)

@Moumouls Moumouls merged commit 66f7af9 into parse-community:master Oct 1, 2020
@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

@timination our ci seems to have some instability and cod cov currently seems to not work properly, on another PR i added some new tests case and we got also a coverage decrease warning.

@Moumouls
Copy link
Member

Moumouls commented Oct 1, 2020

If you want to be sure about tests, the reopen ci action is currently running: https://github.com/parse-community/parse-server/pull/6922/checks?check_run_id=1194629325

@trimination
Copy link
Contributor Author

I'll check it out, thanks @Moumouls =)

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

Successfully merging this pull request may close these issues.

2 participants