Skip to content

Feature/update types#138

Merged
IkeHunter merged 9 commits into
mainfrom
feature/update-types
Sep 19, 2025
Merged

Feature/update types#138
IkeHunter merged 9 commits into
mainfrom
feature/update-types

Conversation

@IkeHunter
Copy link
Copy Markdown
Collaborator

@IkeHunter IkeHunter commented Sep 17, 2025

  • Remove types package
  • Re-add logic for refreshing spotify account
  • Add second test database to isolate db in test
  • Add some routes to assist frontend (join juke session, get current session membership, etc)
  • Update DTOs to match needs of frontend

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation was removed from this. It does not need expose because it is an incoming DTO

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this? I thought this worked already.

return this.jukeSessionService.createMembership(id, body)
}

@Post(':id/members/join/')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the endpoint above this one does the same thing?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "join" endpoint is specifically for the current user, whereas the basic create endpoint just arbitrarily adds any user. We can prob simplify it in the future to make the user id in the body optional, and if it is, set it to the current user

return this.jukeSessionService.endSession(id)
}

@Get(':id/membership/')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to have jukebox_id as a param for authorization.

@Get()
@ApiOperation({ summary: '[MEMBER] Find all jukeboxes for a club id' })
findAll(@Query('clubId', new NumberPipe('clubId')) clubId: number) {
findAll(@Query('club_id', new NumberPipe('clubId')) clubId: number) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pipe param should be changed for consistency as well. Also not sure if the guard works with snake case

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the guard to use snake case

Comment thread src/spotify/spotify.controller.ts Outdated
@Delete('accounts/:id/')
@UseInterceptors(AuthInterceptor)
async deleteSpotifyLink(@Param('id', new NumberPipe('id')) id: number) {
async deleteSpotifyLink(@CurrentUser() user: UserDto, @Param('id') id: number) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use number pipe for validation. UserDto should have expose() decorators replaced with validators for dynamic error handling to function correctly.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs validation on create and expose on TrackDto. Also expose only works if we exclude all fields by default, otherwise it is useless

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean about validation?

@IkeHunter IkeHunter enabled auto-merge September 19, 2025 12:17
@IkeHunter IkeHunter disabled auto-merge September 19, 2025 16:14
@IkeHunter IkeHunter enabled auto-merge September 19, 2025 16:14
@IkeHunter IkeHunter merged commit d9c55b2 into main Sep 19, 2025
3 checks passed
@IkeHunter IkeHunter deleted the feature/update-types branch September 19, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants