Redirection to Strapi endpoints for mobile app users #1641
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…essages on mobile users to the new Strapi API endpoints
|
This pull request has been linked to Shortcut Story #19579: Redirection to Strapi endpoint for mobile app users. |
BrendanGalloway
requested changes
Sep 19, 2023
There was a problem hiding this comment.
The static json file is being moved out of the versioning system to an external location. Is there any synchronisity between the contents of the json file and the version of the mobile/main site? How are updates to the json file captured, reviewed and deployed?
| @@ -96,6 +96,8 @@ spec: | |||
| value: "varnish-{{ .Values.deployEnv }}-{{ .Release.Revision }}" | |||
| - name: SEARCH_HOST | |||
| value: "{{ .Values.nginx.SEARCH_HOST }}" | |||
| - name: STRAPI_LOCATION | |||
| value: "{{ .Values.localSettings.STRAPI_LOCATION }}" | |||
There was a problem hiding this comment.
This needs
- For .Values.localSettings.STRAPI_LOCATION to be defined in the values.yaml file of the chart. You can populate with a sensible default if all Sefaria environments will redirect to the same endpoint
- per-environment values to populated in the environment values files - build/ci/production-values.yaml for prod, build/ci/sandbox-values.yaml for per-commit tests, build/ci/integration-values.yaml for integration tests and create-cauldron.sh in the cauldrons repo for cauldrons.
BrendanGalloway
approved these changes
Sep 19, 2023
|
Tested it on the mobile app using the cauldron URL and it worked |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Story details: https://app.shortcut.com/sefaria/story/19579
Forewarning
This is likely wrong lol. I only tested the redirection part in an example Nginx Docker container on my local. I don't know the correct location or way to reference the secret environment variable used to define
STRAPI_LOCATION. Since it's only for production, the port shouldn't be necessary.Background
I integrated Strapi CMS into Sefaria's codebase. It's currently used on the web interface to serve the content for modals, banners, and sidebar ads. The mobile app currently uses static JSON files for that content which the site needs to redeployed every time when a change needs to be made (see here). To remove the last remaining dependency of the old interrupting message code and ensure backwards compatibility with current mobile app users, there needs to be redirects to the new API endpoints. Having these redirects should work, because the fetch call in the mobile app should default to following redirects though it isn't explicitly specified.
Requirements (What I was trying to accomplish)
Further considerations
In the future, the mobile app's URLs for the endpoints will be changed to use the new Strapi based ones, but those will be included in a new update.
Conclusion
I need to have this done soon (likely by the end of this week), so I tried to get the ball rolling for you guys (@edamboritz and @BrendanGalloway) even though I have no idea what I'm doing :)