-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add registration attributes and guards #171
base: stage
Are you sure you want to change the base?
Conversation
…rds for registrationType
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Just a bit more changes @sharmaigne . Thanks!
backend/usecase/discount_usecase.py
Outdated
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: No discounts for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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.
Should be error NOT_FOUND
backend/usecase/discount_usecase.py
Outdated
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: No discounts for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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.
Should be error NOT_FOUND
backend/usecase/discount_usecase.py
Outdated
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: No discounts for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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.
Should be error NOT_FOUND
event = EventUsecase.get_event( | ||
event_id | ||
) # NOTE: am i doing this right, i don't wanna add another param but di pud ko sure if fetching the whole thing is the best move? |
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.
Use the EventRepository for this.
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: No evaluations for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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.
Should be NOT_FOUND
@@ -45,6 +46,12 @@ def create_registration(self, registration_in: RegistrationIn) -> Union[JSONResp | |||
Union[JSONResponse, RegistrationOut]: If successful, returns the created registration entry. | |||
If unsuccessful, returns a JSONResponse with an error message. | |||
""" | |||
|
|||
event = EventUsecase.get_event(registration_in.eventId) |
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.
Use EventRepository for this. Apply for all instances. Use cases should call repositories for data fetching.
Can you please resolve merge conflicts @sharmaigne ? |
…into 158-be-add-redirect-register-option
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.
More changes needed @ASPactores
Also lease add guards for the following:
- certificate usecase
- preregistration_usecase
Thanks!
event = EventUsecase.get_event(DiscountIn.eventId) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: Discounts should not be created for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.NOT_FOUND, content={'message': message}) |
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 should BAD_REQUEST
.
Should be BAD_REQUEST
for CUD operations and NOT_FOUND
for READ operations
@@ -33,6 +35,13 @@ def create_evaluation(self, evaluation_list_in: EvaluationListIn) -> Union[JSONR | |||
""" | |||
event_id = evaluation_list_in.eventId | |||
registration_id = evaluation_list_in.registrationId | |||
|
|||
# NOTE: next three lines copy pasted a lot, should this be a helper or nah kay mubo ra |
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.
Remove comment
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: No evaluation to update for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.NOT_FOUND, content={'message': message}) |
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 should BAD_REQUEST.
Should be BAD_REQUEST for CUD operations and NOT_FOUND for READ operations
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: No discounts for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.NOT_FOUND, content={'message': message}) |
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 should BAD_REQUEST.
Should be BAD_REQUEST for CUD operations and NOT_FOUND for READ operations
@@ -228,6 +240,11 @@ def get_registration(self, event_id: str, registration_id: str) -> Union[JSONRes | |||
|
|||
""" | |||
|
|||
event = EventUsecase.get_event(event_id) | |||
if event.registrationType == RegistrationType.REDIRECT: |
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 should NOT_FOUND.
Should be BAD_REQUEST for CUD operations and NOT_FOUND for READ operations
event = EventsRepository.query_events(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Error: Evaluation should not be created for REDIRECT registrationType' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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 returns an error since
EventsRepository.query_events
returns a tuple and not the event entry - Use
self.__events_repository
for accessing the entry not the usecase - check if event exists first before accessing the
registrationType
attribute so doif event and event.registrationType
APPLY FOR ALL INSTANCES
event = EventsRepository.query_events(registration_in.eventId) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'Registrations should not be created for REDIRECT registration type' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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.
- Use
self.__events_repository
for accessing the entry not the usecase - Check if event exists first before accessing the
registrationType
attribute so doif event and event.registrationType
APPLY FOR ALL INSTANCES
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.
Please add guards to create_registration_approval_flow
also
@@ -264,6 +281,11 @@ def get_registration_by_email(self, event_id: str, email: str) -> RegistrationOu | |||
registrations, | |||
message, | |||
) = self.__registrations_repository.query_registrations_with_email(event_id=event_id, email=email) | |||
event = EventUsecase.get_event(event_id) | |||
if event.registrationType == RegistrationType.REDIRECT: |
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 should NOT_FOUND.
Should be BAD_REQUEST for CUD operations and NOT_FOUND for READ operations
event = EventUsecase.get_event(event_id) | ||
if event.registrationType == RegistrationType.REDIRECT: | ||
message = 'No registrations for REDIRECT registration type' | ||
return JSONResponse(status_code=HTTPStatus.BAD_REQUEST, content={'message': message}) |
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 should NOT_FOUND.
Should be BAD_REQUEST for CUD operations and NOT_FOUND for READ operations
No description provided.