-
Notifications
You must be signed in to change notification settings - Fork 0
Feature | TRUSPD-583 | 404, api route, events member wide update #1
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
Feature | TRUSPD-583 | 404, api route, events member wide update #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@echou-truvolv looking good! i left a couple comments i'd be curious your thoughts on, let me know what ya think!
| # Step 5: Update API route, the route should exist | ||
| if [[ -f "$app_dir/app/api/[...slug]/route.ts" ]]; then | ||
| echo "Updating API route in $app_name/app/api/[...slug]/route.ts" | ||
| # Replace all content with new API route content | ||
|
|
||
| cat > "$app_dir/app/api/[...slug]/route.ts" << 'EOF' | ||
| // exporting all route handlers from orson-seelib | ||
| // we can overwrite specific handlers here per member if needed | ||
| export * from "@truvolv/orson-seelib/api/router"; | ||
| EOF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i could be mistaken but i think im reading the logic up to now that it only skips updating the router handler if it already exactly matches the export * line format (ie was updated already). i know for sure the Cyprus has a custom route handler that we would not want to be updated / overwritten and there might be a couple other niche member repos in that boat.
For the Live Preview one, i checked if the member site's middleware perfectly matched the middleware that was previously in orson-template and only then rewrote the file. If there were any differences (ex Jacuzzi's middleware is custom), i flagged that in the script in the logs that it needed a manual update and the script passed over that file. I'm thinking that might be a good rule of thumb for these kinds of changes, checking first the it's a normal instance of a file and bailing if there's anything different at all.
What're your thoughts there / am i reading it right up until now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A cool future state, def doesn't have to be right now, would maybe be having a script that takes an array of like
- file path
- what the current orson-template file is (ie what to match on to check if it's eligible to update or if the member has custom logic we need to look out for)
- what the updated file is (used to make the update and also check if it's actually been updated)
That might be a way to make it generic / reusable with minimal edits needed by future us 🤔
Again totally doesn't need to be a right now thing but could be neat thing to work towards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh yeah that's a good flag! i updated the script to only overwrite the file if it matches the existing orson template structure. i also added a manual-update.txt file that'll have all of the manual edits we'll need to make and will update the spreadsheet with a sheet for that info so we can keep track of it there!
the output when running on different cyprus apps

i also added a clause to only update the core events page if it doesn't exist, this should really only apply to bath experts

to your other point, i like that idea!! i created a ticket here so we can have that in the back of our minds when we have the time to dig deeper into that logic. feel free to add any other ideas / edit anything as you see fit!
| --body "This PR updates: | ||
|
|
||
| - API route to include all handlers from orson-seelib | ||
| - 404 page skeleton to use NotFoundPage component from orson-seelib | ||
| - Events page to display separate events page using CoreEvent component from orson-seelib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is so small but could we include a ticket link in the PR?
| @@ -0,0 +1,152 @@ | |||
| ## 🧩 Repository Batch Updater Script | |||
|
|
|||
| This script was written for batch updates across our member repos. In this instance, the intent was to update the NextJS version and to make the middleware asynchronous. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this might be leftover from copy/pasting?
Description
QA
Optionally
repo-list.tsxfile with the repos you want to test.bash update-repos.sh.