Skip to content

Conversation

@commonly-ts
Copy link
Contributor

@commonly-ts commonly-ts commented Nov 20, 2025

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

  • ✨ New feature (non-breaking change which adds functionality)
  • ✅ Test update

Changes Made

  • Adds placeId parameter to universes.updateUserRestriction
  • Updates universes.updateUserRestriction to use an options object for universeId, placeId and body.
  • Adds placeId check to the updateUserRestriction test.

Testing

  • All existing tests pass (pnpm test)
  • Added new tests for new functionality
  • Tested manually (describe below)

Manual Testing

Simple string concatenation so no manual testing needed

Code Quality

  • My code follows the project's coding style
  • I have run pnpm lint and fixed any issues
  • I have run pnpm format to format my code
  • I have run pnpm typecheck and there are no type errors
  • I have reviewed my own code

Documentation

  • I have updated the relevant documentation (if applicable)
  • I have added/updated JSDoc comments for new/modified code

Screenshots/Recordings

image

Checklist

  • I have read the CONTRIBUTING.md guide
  • My PR title follows the Conventional Commits format
  • I have rebased my branch on the latest main branch
  • I have tested my changes against the Roblox Open Cloud API (if applicable)
  • This PR is ready for review

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
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/resources/universes.ts 91.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@commonly-ts
Copy link
Contributor Author

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

Copy link
Member

@marinofranz marinofranz left a 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

@marinofranz marinofranz added the enhancement New feature or request label Nov 20, 2025
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.
Copy link
Member

@marinofranz marinofranz left a 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 🙂

@commonly-ts
Copy link
Contributor Author

commonly-ts commented Nov 23, 2025

also wondering about readability for duration as its not particularly clear that it needs to be in seconds with "s" at the end, unless you go and read the API docs.

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.
@marinofranz
Copy link
Member

also wondering about readability for duration as its not particularly clear that it needs to be in seconds with "s" at the end, unless you go and read the API docs.

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?

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 🙂
There's just one missing test that should test if duration is an empty string to pass coverage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants