-
Notifications
You must be signed in to change notification settings - Fork 20
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
23504 - Endpoint for cc confirmation. #281
Conversation
from search_api.services.document_services.document_access_request import update_document_access_request_status_by_id | ||
from search_api.services.gcp_auth.auth_service import ensure_authorized_queue_user | ||
|
||
bp = Blueprint('GCP_LISTENER', __name__) # pylint: disable=invalid-name |
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.
return {}, HTTPStatus.BAD_REQUEST | ||
|
||
try: | ||
credit_card_payment = ce.data |
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.
Would put in another check for corp_type
, just in case
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.
done
search-api/requirements.txt
Outdated
git+https://github.com/daxiom/simple-cloudevent.py.git | ||
git+https://github.com/daxiom/flask-pub.git@0.0.4 | ||
git+https://github.com/bcgov/sbc-connect-common.git@43411ed428c4c4b89bea1ac6acdb10077f247d2b#egg=gcp_queue&subdirectory=python\gcp-queue |
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.
I think this line installs 66 and 67 has?
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.
true, removed explicit line in requirements.txt
@@ -62,5 +62,7 @@ six==1.16.0 | |||
strict-rfc3339==0.7 |
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.
you need to add changes here into the appropriate requirements/... file. Otherwise next time someone builds the reqs to upgrade everything it will remove these
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.
Fixed the issues in requriements.txt
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.
I'd recommend upgrading to Poetry :P
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.
CI is failing and definitely needs some tests and I don't see any config updates to support the new variables you're referencing (i.e. PAY_AUDIENCE_SUB)
@@ -0,0 +1,5 @@ | |||
class DbRowNotFound(Exception): |
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.
put this in the exceptions file with all the other ones using the base exception pls
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.
Done
raise DbRowNotFound() | ||
|
||
dar.status = status | ||
dar.save() |
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.
pls keep it in the resource (its a couple lines of code) or put it in request_handlers/document_request_handlers (that's where the other similar functions are)
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.
OK, moved it into the resourece
|
||
try: | ||
credit_card_payment = ce.data | ||
if credit_card_payment.get('corpTypeCode', '') != 'BUS': |
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.
easier if you just convert it to an object / dataclass I think
try: | ||
credit_card_payment = ce.data | ||
if credit_card_payment.get('corpTypeCode', '') != 'BUS': | ||
raise Exception('invalid or missing corpTypeCode.') # noqa: E713 # pylint: disable=broad-exception-raised |
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.
do you guys have BusinessException? maybe craft another one for that?
/gcbrun |
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.
Looks good to me 👍
*Issue #:23504 * /bcgov/entity#23504
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the PPR license (Apache 2.0).