Conversation
If we're putting the method and such in the JSON object, it makes sense to keep the headers at that layer rather than pushing them up to gRPC. This lets the Express router access those headers in the usual way.
Easy mistake
This worked well for miiverse-api. It still just spits the stacktrace into the browser, but hey, at least it doesn't crash the server process
i'm not stupid i'm smart
some callsites to the old database method still exist, since the helpers on Post are used for things the backend doesn't have yet it's annoying to pass the whole Express request so just have a blob object for the things that the user is authenticated with
mrjvs
requested changes
Jun 8, 2025
apps/juxtaposition-ui/src/services/juxt-web/routes/console/posts.js
Outdated
Show resolved
Hide resolved
apps/juxtaposition-ui/src/services/juxt-web/routes/console/posts.js
Outdated
Show resolved
Hide resolved
apps/miiverse-api/src/services/internal/middleware/authentication.ts
Outdated
Show resolved
Hide resolved
apps/miiverse-api/src/services/internal/middleware/check-user-account.ts
Outdated
Show resolved
Hide resolved
apps/miiverse-api/src/services/internal/middleware/authentication.ts
Outdated
Show resolved
Hide resolved
apps/miiverse-api/src/services/internal/middleware/check-user-account.ts
Outdated
Show resolved
Hide resolved
mrjvs
requested changes
Jun 8, 2025
Member
Author
|
Note to self: Need to check Juxt bans on the API too (not just PNID bans) |
I went through a copy of the prod db and found lots of optional fields that aren't marked as such, old documents missing fields, etc. Introduces an IPostInput type that accounts for the mongoose schema doing defaults and fixes, while also allowing IPost to reflect the real db type. Groundwork for a proper DTO structure.
apps/juxtaposition-ui/src/services/juxt-web/routes/web/login.js
Dismissed
Show dismissed
Hide dismissed
Member
Author
|
I hate CodeQL actually. I do exactly what it suggests and it goes "nuh" |
This provides a layer of abstraction between the actual database implementation and the on-the-wire types which will be useful when the database is changed soon
Mostly naming tweaks for clarity
mrjvs
requested changes
Jun 12, 2025
mrjvs
requested changes
Jun 13, 2025
Some paths use the login template to render errors (?) and this now requires a redirect URL. Provide one.
mrjvs
approved these changes
Jun 15, 2025
Contributor
|
lgtm, is this ready for merge? |
Member
Author
|
I think so. I am a bit worried about token expiry (web frontend requests that go thru this api check the token on every request now) but we can always branch this back off if it becomes a problem |
Member
Author
|
Token expiry issue PretendoNetwork/account#184 |
Contributor
|
This is waiting on PretendoNetwork/account#184 to be resolved, can be merged when that's done |
Member
Author
|
Account server side is good to go, ~~now just need our side ( |
Member
Author
|
This should be ok to go barring any conflict with #138 |
This file contains hidden or 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
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.
Part of #112
Need to submit this before it gets unreviewable
Changes:
This PR adds the extra infrastructure needed for miiverse-api to act as datasource (authentication, error handling, type conversions, validation etc.) and then moves the "post by id" endpoint (and only this endpoint) to the new backend. A few calls to the old database exist in the frontend since they rely on the Post being a Mongoose object for further DB calls - these will be ported once those further calls are in the API.
I know this is an absolute boatload of infrastructure for a very minimalist endpoint (and even it's missing some things, like the moderator/removed posts override) but hopefully it will serve as a foundation for us to work on top of.