-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix: dashboard various fixes #78
Changes from all commits
bc5e38c
ffe6c3a
114a081
0832ac3
0d875d1
2299023
5c00a65
cb1d5db
8a0ad34
d2476fe
f4bd076
4929924
d20e210
618327c
edd1887
f49e0be
33fdccc
accdd12
8f6d6d8
54011ba
859906c
0854c72
dfea6a6
e7bf445
1b1966d
f4612b4
0fa2298
47a2e7f
c6d93b3
b7c5ab1
952eb25
05753e7
969b532
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { Test, TestingModule } from '@nestjs/testing'; | ||
import { ConnectionsController } from './connections.controller'; | ||
import { ConnectionsService } from './connections.service'; | ||
import { PrismaService } from '../../src/prisma/prisma.service'; | ||
|
||
describe('ConnectionsController', () => { | ||
let controller: ConnectionsController; | ||
|
||
beforeEach(async () => { | ||
const module: TestingModule = await Test.createTestingModule({ | ||
controllers: [ConnectionsController], | ||
providers: [ConnectionsService, PrismaService], | ||
}).compile(); | ||
|
||
controller = module.get<ConnectionsController>(ConnectionsController); | ||
}); | ||
|
||
it('should be defined', () => { | ||
expect(controller).toBeDefined(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
import { Controller, Delete, Get, Param, Post } from '@nestjs/common'; | ||
import { CurrentUser } from '../../src/decorators/current-user.decorator'; | ||
import { ConnectionsService } from './connections.service'; | ||
import { ConnectionDto } from './dto/Connection.dto'; | ||
import { | ||
ApiBadRequestResponse, | ||
ApiBearerAuth, | ||
ApiNotFoundResponse, | ||
} from '@nestjs/swagger'; | ||
import { User } from '@prisma/client'; | ||
import { Public } from '../../src/decorators/public.decorator'; | ||
|
||
@ApiBearerAuth() | ||
@Controller('connections') | ||
export class ConnectionsController { | ||
constructor(private connectionsService: ConnectionsService) {} | ||
/** | ||
* | ||
* Get all of the current user's connections | ||
*/ | ||
@Get() | ||
async getAllConnections(@CurrentUser() user: User): Promise<ConnectionDto[]> { | ||
return this.connectionsService.getUserConnections(user.id); | ||
} | ||
|
||
/** | ||
* Get "suggested for review" connections. | ||
*/ | ||
@Get('/suggested') | ||
async getSuggestedConnections( | ||
@CurrentUser() user: User, | ||
): Promise<ConnectionDto[]> { | ||
return this.connectionsService.getUserConnections(user.id); | ||
} | ||
|
||
/** | ||
* Search an user by query. | ||
*/ | ||
@Get('/search/:query') | ||
@Public() | ||
@ApiBadRequestResponse() | ||
async searchUser( | ||
@Param('query') query: string, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you can add an alphanumeric regex check in here so that users cant use SQL injection? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If prisma wouldn't be Sql injection safe out of the box all of our endpoint would be vulnerable for such a thing since we're not doing any kind of prevention for this anywhere. |
||
@CurrentUser() user?: User, | ||
): Promise<ConnectionDto[]> { | ||
return this.connectionsService.searchUsers(user?.id, query); | ||
} | ||
|
||
/** | ||
* Create a connection between current User and another user. | ||
*/ | ||
@Post('/connect/:userId') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a better name for this would be
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way it is right now it is in alignment with RESTful standards. The verb "POST" signifies a resource creation, and DELETE signifies a resource removal. https://mglaman.dev/blog/post-or-put-patch-and-delete-urls-are-cheap-api-design-matters There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, I know it is bit of a gamble in here. The thing is, feels that the |
||
async connectWithUser( | ||
@CurrentUser() user: User, | ||
@Param('userId') userId: string, | ||
): Promise<ConnectionDto> { | ||
return this.connectionsService.addConnection(user.id, userId); | ||
} | ||
/** | ||
* | ||
* Remove connection between current user and another user. | ||
*/ | ||
@Delete('/connect/:userId') | ||
async unconnectWithUser( | ||
@CurrentUser() user: User, | ||
@Param('userId') userId: string, | ||
): Promise<ConnectionDto> { | ||
return this.connectionsService.removeConnection(user.id, userId); | ||
} | ||
|
||
/** | ||
* Search for users by external profile URL | ||
* @param profileUrlBase64 | ||
*/ | ||
|
||
@Public() | ||
@Get('search-by-external-profile/:profileUrl') | ||
async searchUsersByExternalProfile( | ||
@Param('profileUrl') profileUrlBase64: string, | ||
) { | ||
return this.connectionsService.searchUserByExternalProfile( | ||
profileUrlBase64, | ||
); | ||
} | ||
|
||
/** | ||
* Get a connection. | ||
*/ | ||
@Get('/:userId') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure about this usecase. Can you link a screenshot/url? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It gets a single "connection" https://www.figma.com/design/p4gfqRDn3ma4zPaPAj8vH4/Culero?node-id=46-2936&t=pqBwKPkw3kB9HieJ-4 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay! |
||
@ApiNotFoundResponse() | ||
async getUser( | ||
@CurrentUser() user: User, | ||
@Param('userId') userId: string, | ||
): Promise<ConnectionDto> { | ||
return this.connectionsService.getConnection(userId, user.id); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { Module } from '@nestjs/common'; | ||
import { ConnectionsService } from './connections.service'; | ||
import { ConnectionsController } from './connections.controller'; | ||
|
||
@Module({ | ||
providers: [ConnectionsService], | ||
controllers: [ConnectionsController], | ||
}) | ||
export class ConnectionsModule {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { Test, TestingModule } from '@nestjs/testing'; | ||
import { ConnectionsService } from './connections.service'; | ||
import { PrismaService } from '../../src/prisma/prisma.service'; | ||
|
||
describe('ConnectionsService', () => { | ||
let service: ConnectionsService; | ||
|
||
beforeEach(async () => { | ||
const module: TestingModule = await Test.createTestingModule({ | ||
providers: [ConnectionsService, PrismaService], | ||
}).compile(); | ||
|
||
service = module.get<ConnectionsService>(ConnectionsService); | ||
}); | ||
|
||
it('should be defined', () => { | ||
expect(service).toBeDefined(); | ||
}); | ||
}); |
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.
Our existing endpoints use singular in the controller name, so we should be sticking to
connection
in here.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.
This is a larger discussions. I think it makes sense to have
user
since in that controller we're usually dealing with a single user.Put in
reviews
andconnections
we are usually dealing with multiple resources. See: Google's api design guidelinesand
this StackOverflow discussion
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.
This makes sense