Let Content Admin edit starter quests & Add Academy to Village menus#1378
Let Content Admin edit starter quests & Add Academy to Village menus#1378Phrosfire wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
WalkthroughThe PR updates existing Academy ChangesAcademy Village Structure, Navigation, and Permission
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
size:XXL
Confidence Score: 5/5Safe to merge — changes are small, intentional, and well-scoped. The only non-trivial aspect is the broadened starter-quest permission, which is acknowledged in the PR description. All four changed areas are straightforward: a one-line permission list addition, a one-line nav option addition, a single-statement migration, and the matching seed-file update. No schema changes, no new endpoints, and the existing server-side guard continues to protect the tutorial quest's hidden field. The gap in tutorial-quest coverage (content/objectives remain editable by CONTENT-ADMIN) is a deliberate trade-off documented in the PR description rather than an unintended regression. No files require special attention. The permission change in permissions.ts is the most consequential edit; the server-side enforcement context in quests.ts was reviewed to confirm no unintended access paths are opened. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Quest Edit Request] --> B{canChangeContent?}
B -- No --> C[Reject]
B -- Yes --> D{Is starter quest?}
D -- No --> E[Allow edit]
D -- Yes --> F{canEditStarterQuests?}
F -- No --> G[Reject: Not allowed]
F -- Yes\nOWNER or CONTENT-ADMIN --> H{Is tutorial quest ID\nAND hidden=true?}
H -- Yes --> I[Reject: Cannot edit tutorial quest]
H -- No --> J[Allow edit\nincl. content & objectives]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Quest Edit Request] --> B{canChangeContent?}
B -- No --> C[Reject]
B -- Yes --> D{Is starter quest?}
D -- No --> E[Allow edit]
D -- Yes --> F{canEditStarterQuests?}
F -- No --> G[Reject: Not allowed]
F -- Yes\nOWNER or CONTENT-ADMIN --> H{Is tutorial quest ID\nAND hidden=true?}
H -- Yes --> I[Reject: Cannot edit tutorial quest]
H -- No --> J[Allow edit\nincl. content & objectives]
Reviews (2): Last reviewed commit: "fix migration" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
/tnr-review-now |
1. Scope covered
2. Test steps executed
3. Findings (pass/fail, bugs, risks)
4. Screenshot index
5. Recommendation
Screenshotsacademy page player mobile nav academy configured mobile nav settings open player profile login village academy visible Run: View workflow run |
MathiasGruber
added
the
ApprovedButHasMigrations
MathiasGruber
left a comment
MathiasGruber
left a comment
There was a problem hiding this comment.
Logic and DRY review
| ('wY4rItGdX9-AcUpOdF1S', 'Auction House', 'https://ui0arpl8sm.ufs.sh/f/Hzww9EQvYURJmcDNSqHE4IMO5Goa7cgLxPJ0VC6lU8vbt1Ap', 'XN9oEb5tKAv', '1', '1', '100', '100', '6', '12', '1', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '1', '/auctionhouse', '2026-03-17 00:00:00.000', '1'), | ||
| ('rHN3laRV2C-df7kkGGFK', 'Auction House', 'https://ui0arpl8sm.ufs.sh/f/Hzww9EQvYURJmcDNSqHE4IMO5Goa7cgLxPJ0VC6lU8vbt1Ap', 'ryBk0qD4EgvPPyav2K4OC', '1', '1', '100', '100', '6', '12', '1', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '1', '/auctionhouse', '2026-03-17 00:00:00.000', '1'); | ||
|
|
||
| UPDATE `VillageStructure` SET `showInVillagePage` = true WHERE `route` = '/academy'; |
There was a problem hiding this comment.
This UPDATE is a no-op against the seed data: the Academy row inserted at line 146 already has showInVillagePage = '1' as its final column value. The migration (0018_silly_beast.sql) is needed to backfill production, but in the seed file this statement just duplicates state that the INSERT already established. Consider removing it, or alternatively flip the seed insert to 0` so the UPDATE meaningfully exercises the same code path as production.
"Completed" label removedThis PR has 1 unresolved review thread(s). The "Completed" label has been automatically removed. Please resolve all review threads before re-applying the label. Unresolved threads:
|
3 participants
Pull Request
Update permission to allow content admin to edit starter quests. This is so he can add new starter quests that take place after the tutorial is completed and the user is already a genin. He is well aware that the tutorials itself should not be touched. The Academy is also being added to the village menu.
License
By making this pull request, I confirm that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the Studie-Tech ApS organization has the copyright to use and modify my contribution for perpetuity.
Summary by CodeRabbit
/academy.CONTENT-ADMINrole (in addition toOWNER).