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.

Sign up for GitHub

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

Jump to bottom

feat(android, iOS)!: require appId for instagram stories to meet new Meta requirement #1305

Merged
merged 3 commits into from Nov 3, 2022
Merged

feat(android, iOS)!: require appId for instagram stories to meet new Meta requirement #1305

merged 3 commits into from Nov 3, 2022

Conversation

rickafds
Copy link
Contributor

mikehardy
mikehardy requested changes Nov 1, 2022
View reviewed changes
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Thank you for posting this! Seems reasonable, just a question about failing fast for both platforms

android/src/main/java/cl/json/social/InstagramStoriesShare.java Outdated Show resolved Hide resolved
mikehardy
mikehardy approved these changes Nov 1, 2022
View reviewed changes
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

I like it! Fail fast. We'll have to do a major release on this since it will break existing code, but that is totally justified + okay to do

@mikehardy mikehardy changed the title feat(android, iOS): Support source application for instagram stories feat(android, iOS)!: require appId for instagram stories to meet new Meta requirement Nov 1, 2022
@mikehardy
Copy link
Collaborator

@MateusAndrade this looks good to me - I put a ! in the PR title so it should 🤞 trigger a major release, with the PR message indicating what exactly the break is. I read the related docs on the issue when it first came through, this all seems 💯 to me

@rickafds --> 🏆 ! Thank you for the contribution

@ZerakPalani
Copy link

@rickafds Thank you for contributing!

@MateusAndrade MateusAndrade merged commit ea99612 into react-native-share:main Nov 3, 2022
MateusAndrade pushed a commit that referenced this pull request Nov 3, 2022
@rickafds
feat(instagram): require appId for instagram stories to meet new Meta…
20b270b 
… requirement (#1305)

* feat(android): support added for instagram source application

* feat(ios): support added for instagram source application

* feat(android): valid error in the javascript

BREAKING CHANGE: require appId according to Meta updates
MateusAndrade pushed a commit that referenced this pull request Nov 3, 2022
@semantic-release-bot
chore(release): 8.0.0 [skip ci]
44af93a 
# [8.0.0](v7.9.1...v8.0.0) (2022-11-03)

### Features

* **instagram:** require appId for instagram stories to meet new Meta requirement ([#1305](#1305)) ([20b270b](20b270b))

### BREAKING CHANGES

* **instagram:** require appId according to Meta updates
@MateusAndrade
Copy link
Collaborator

@MateusAndrade this looks good to me - I put a ! in the PR title so it should 🤞 trigger a major release, with the PR message indicating what exactly the break is. I read the related docs on the issue when it first came through, this all seems 💯 to me

@rickafds --> 🏆 ! Thank you for the contribution

I forgot to merge this with the breaking change pattern, so I did an ammend on the commit and now I'm a co-author with @rickafds. 😅

Btw, thanks for the amazing work @rickafds! 🚀 🚀 🚀

mobiledev7 added a commit to mobiledev7/react-native-share that referenced this pull request Dec 12, 2022
@mobiledev7
chore(release): 8.0.0 [skip ci]
e51c88c 
# [8.0.0](react-native-share/react-native-share@v7.9.1...v8.0.0) (2022-11-03)

### Features

* **instagram:** require appId for instagram stories to meet new Meta requirement ([#1305](react-native-share/react-native-share#1305)) ([20b270b](react-native-share/react-native-share@20b270b))

### BREAKING CHANGES

* **instagram:** require appId according to Meta updates
BrodaNoel
BrodaNoel reviewed Dec 21, 2022
View reviewed changes
src/types.ts
@@ -74,6 +74,7 @@ interface BaseSocialStoriesShareSingleOptions extends Omit<BaseShareSingleOption

export interface InstagramStoriesShareSingleOptions extends BaseSocialStoriesShareSingleOptions {
social: Social.InstagramStories;
appId: string;
Copy link

Choose a reason for hiding this comment

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

Can somebody add more info here? What appId? Should this be the Facebook App Id? Or the AppStore app id?

Copy link

Choose a reason for hiding this comment

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

According to Facebook's docs on sharing to Instagram stories, the id is the Facebook app id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrodaNoel BrodaNoel BrodaNoel left review comments

@alis0nc alis0nc alis0nc left review comments

@mikehardy mikehardy mikehardy approved these changes

@jgcmarins jgcmarins Awaiting requested review from jgcmarins jgcmarins is a code owner

@MateusAndrade MateusAndrade Awaiting requested review from MateusAndrade MateusAndrade is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@rickafds @mikehardy @ZerakPalani @MateusAndrade @BrodaNoel @alis0nc