Added session feedback controller and service with tests for error an… #520
Conversation
| @Injectable() | ||
| export class SessionFeedbackService { | ||
| constructor( | ||
| @InjectRepository(PartnerAccessEntity) |
There was a problem hiding this comment.
I am not exactly sure why all these repos are needed, as it does not seem to be the case for other services in this repo, but I had to add all of these to get rid of any errors created. The only other service that this service directly depends on is the session service - the rest of the dependencies are just transitive
There was a problem hiding this comment.
It's a great question and you are right to pick it up. It is something we are slowly trying to fix. The issue is the function getUserByFirebaseId in the userService. It does waayyyyyy too much. It's unfortunately used in the AuthGuard. The FirebaseAuthGuard is used across all the services. What we are trying to do is start to divide up how we fetch the data and ensure everything is fetched separately. See this issue. It's going to be a slow process.
| @ApiProperty({ | ||
| enum: FEEDBACK_TAGS_ENUM, | ||
| type: String, | ||
| example: Object.values(FEEDBACK_TAGS_ENUM), |
There was a problem hiding this comment.
This will show all the enum values in the swagger "Example Value" section in a list - not sure if this is more or less convenient than it just saying "string"
| describe('SessionFeedbackService', () => { | ||
| let service: SessionFeedbackService; | ||
| let mockPartnerAccessRepository: DeepMocked<Repository<PartnerAccessEntity>>; | ||
| let mockPartnerRepository: DeepMocked<Repository<PartnerEntity>>; |
There was a problem hiding this comment.
Same comment/question as below about all the transitive dependencies for this service
| @@ -0,0 +1,10 @@ | |||
| import { ISession } from 'src/session/session.interface'; | |||
There was a problem hiding this comment.
Could I be a pain and say lets get rid of this interface and just use the SessionFeedbackEntity type in the service?
There was a problem hiding this comment.
Just to clarify, you mean I should completely remove the session-feedback.interface? I think I am only using the SessionFeedbackEntity in the session-feedback.service anyway so this should not be a problem. Thanks!
|
@ckirby19 Thanks for all your comments and I agree with them all. I should have highlighted the new type patterns in the ticket. Apologies. We are only just starting to clean up the backend as it hasn't been a priority! I have one small request to remove the ISessionFeedback which you highlighted. Otherwise, this is good to go. |
…ttps://github.com/ckirby19/bloom-backend into feature/ticket-396-add-session-feedback-controller
|
Brilliant - thanks @ckirby19 ⭐ |
…d success cases
Issue link / number:
#396
What changes did you make?
Why did you make the changes?
As requested on the ticket
Did you run tests?
Yes, I added new tests for the success and error cases in the service, and these tests were successful. I was also able to hit the endpoint with a POST request using postman and have a new session feedback entity be added to the session feedback database table