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

fix: App link intent appear when clicking on Terms and Policy Section #2345

Merged
merged 2 commits into from
Sep 16, 2019
Merged

Conversation

rob729
Copy link
Contributor

@rob729 rob729 commented Sep 14, 2019

Fixes #2292

Changes: Changed the DEFAULT_BASE_URL for release build

GIF for the change:

@auto-label auto-label bot added the fix label Sep 14, 2019
Copy link
Member

@iamareebjamal iamareebjamal left a comment

Choose a reason for hiding this comment

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

?

@rob729
Copy link
Contributor Author

rob729 commented Sep 14, 2019

?

Is there any problem?

app/build.gradle Outdated
@@ -60,7 +60,7 @@ android {
buildConfigField "String", "MAPBOX_KEY", '"'+MAPBOX_KEY+'"'
buildConfigField "String", "STRIPE_API_KEY", '"'+STRIPE_API_TOKEN+'"'
buildConfigField "String", "PAYPAL_CLIENT_ID", '"'+PAYPAL_CLIENT_ID+'"'
resValue "string", "FRONTEND_HOST", "eventyay.com"
resValue "string", "FRONTEND_HOST", "api.eventyay.com"
Copy link
Member

Choose a reason for hiding this comment

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

What does it mean?

Copy link
Member

Choose a reason for hiding this comment

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

I told you to change the API URL for reproducing the issue, for debugging.

Copy link
Contributor Author

@rob729 rob729 Sep 14, 2019

Choose a reason for hiding this comment

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

Actually, Default base URL starts with "api.eventyay" and in the intent filter, the host had value "eventyay.com" because of which options of choosing app was coming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the API URL for reproducing the error. Do you want it to remain changed or back to initial value?

Copy link
Member

Choose a reason for hiding this comment

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

You unfortunately, don't understand the issue. Read it again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iamareebjamal @liveHarshit If I am correct then the app deep linking feature is not working for the release build of the app and I am supposed to solve this issue.
Please correct me if I am wrong?

Copy link
Member

@liveHarshit liveHarshit Sep 15, 2019

Choose a reason for hiding this comment

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

Yes, you're. App link intent should accept the only following type of URLs -

  1. Event details: https://eventyay.com/e/{event-identifier}
  2. Reset password: https://eventyay.com/reset-password?token={token}
  3. Verify email: https://eventyay.com/email/verify?token={token}

For other types like terms and policy URLs, it should not take it as app link intent.

But here you removed the whole feature instead. We are not targeting any type of build. I told you to change frontend host only for testing purposes as all links are with eventyay.com host.
Clear??

Copy link
Member

Choose a reason for hiding this comment

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

I changed the API URL for reproducing the error. Do you want it to remain changed or back to initial value?

So we send PR after fixing the error, not after reproducing it.

@rob729
Copy link
Contributor Author

rob729 commented Sep 15, 2019

@liveHarshit @iamareebjamal I have made changes in my PR.
Please review them.

@iamareebjamal
Copy link
Member

iamareebjamal commented Sep 15, 2019

Have you ensured that normal app linking it working as expected?

@rob729
Copy link
Contributor Author

rob729 commented Sep 15, 2019

@iamareebjamal I have checked it for email verification and it was working properly.

@iamareebjamal
Copy link
Member

Please test for event and password reset as well

@rob729
Copy link
Contributor Author

rob729 commented Sep 16, 2019

It is working for all the three cases mentioned above.

@iamareebjamal iamareebjamal merged commit 70773f1 into fossasia:development Sep 16, 2019
ranjsa pushed a commit to ranjsa/open-event-attendee-android that referenced this pull request Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App link intent appear when clicking on Terms and Policy Section
3 participants