-
Notifications
You must be signed in to change notification settings - Fork 2
feat(universes): Add place-level user restriction support #9
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
base: main
Are you sure you want to change the base?
Conversation
Extends the Universes class to allow updating user restrictions at the place level by accepting an optional placeId parameter. Adjusts the resource path logic to handle both universe-level and place-level user restrictions.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
I would agree if the method's parameters are a bit icky now, I'm fine with changing it into options typed with an interface if you'd like |
marinofranz
left a comment
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 agree, let's make this into an options object so we can re-use
There are a couple of other methods that have this feature as well, you can integrate the options object into that as well if you're feeling up for it 🙂
Make sure to add this to a test or create a new one so coverage can pass
Prevents API errors by setting 'duration' to undefined if it is an empty string when updating user restrictions. This avoids sending invalid data that would result in a 400 response from the API.
Refactored Universes.updateUserRestriction to set duration as undefined if an empty string is provided, preventing API 400 errors. Updated related test to verify correct handling of empty duration values.
marinofranz
left a comment
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.
There are a few changes that I want to address regarding the code style, the logic is perfectly fine 🙂
|
also wondering about readability for could rename it to durationSeconds and have it as a number, and then parse it inside the function or is this SDK just a base wrapper without that sort of thing? |
Swapped the order of universeId and userRestrictionId in updateUserRestriction to match API expectations. Updated related type definitions, documentation, and tests for consistency.
We try to stick to just the API reference but we can eventually figure out a helper function for that, this will do for now 🙂 |
Description
Extends the Universes class to allow updating user restrictions at the place level by accepting an optional placeId parameter. Adjusts the resource path logic to handle both universe-level and place-level user restrictions.
Type of Change
Changes Made
placeIdparameter touniverses.updateUserRestrictionuniverses.updateUserRestrictionto use an options object foruniverseId,placeIdandbody.Testing
pnpm test)Manual Testing
Simple string concatenation so no manual testing needed
Code Quality
pnpm lintand fixed any issuespnpm formatto format my codepnpm typecheckand there are no type errorsDocumentation
Screenshots/Recordings
Checklist
mainbranch