Add platform admin page for uuid queries #1965
Conversation
sastels
force-pushed
the
support-id-query
branch
from
a90cdf2
to
7b98b07
Compare
🧪 Review environmenthttps://kxitqumga2jjuzybsfol5rncti0slyhf.lambda-url.ca-central-1.on.aws/ |
sastels
force-pushed
the
support-id-query
branch
from
af22fe3
to
0c8f1f5
Compare
app/config.py
Outdated
| @@ -25,8 +25,7 @@ class Config(object): | |||
| ALLOW_DEBUG_ROUTE = env.bool("ALLOW_DEBUG_ROUTE", False) | |||
|
|
|||
| # List of allowed service IDs that are allowed to send HTML through their templates. | |||
| ALLOW_HTML_SERVICE_IDS: List[str] = [ | |||
| id.strip() for id in os.getenv("ALLOW_HTML_SERVICE_IDS", "").split(",")] | |||
There was a problem hiding this comment.
All the changes in this file were done by make format.
app/utils.py
Outdated
| @@ -93,7 +93,7 @@ def from_lambda_api(line): | |||
| return isinstance(line, dict) | |||
|
|
|||
|
|
|||
| @cache.memoize(timeout=12*60*60) | |||
| @cache.memoize(timeout=12 * 60 * 60) | |||
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
sastels
force-pushed
the
support-id-query
branch
from
30b5349
to
3312f3b
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -165,6 +166,7 @@ def get_locale(): | |||
| provider_client, | |||
There was a problem hiding this comment.
The new import support_api_client should be added in alphabetical order to maintain consistency with the other imports.
app/main/forms.py
Outdated
| @@ -1329,6 +1329,13 @@ class SearchUsersByEmailForm(StripWhitespaceForm): | |||
| ) | |||
|
|
|||
|
|
|||
| class SearchIds(StripWhitespaceForm): | |||
| search = SearchField( | |||
| _l("List of IDs"), | |||
There was a problem hiding this comment.
Consider renaming the label from 'List of IDs' to 'List of UUIDs' to be more specific about the expected input.
| class SearchIds(StripWhitespaceForm): | ||
| search = SearchField( | ||
| _l("List of IDs"), | ||
| validators=[DataRequired(_l("You need to enter one or more UUIDs to search by."))], |
There was a problem hiding this comment.
The validation message 'You need to enter one or more UUIDs to search by.' should be updated to 'You need to enter one or more valid UUIDs to search by.' to clarify that the input should be valid UUIDs.
There was a problem hiding this comment.
That won't make much difference.
| form = SearchIds() | ||
| records = None | ||
| if form.validate_on_submit(): | ||
| records = support_api_client.find_ids(form.search.data) |
There was a problem hiding this comment.
Consider adding error handling for the support_api_client.find_ids call to manage potential exceptions or errors from the API.
There was a problem hiding this comment.
A generally good idea, but in this case if there's a server error with api then it's fine for the admin web page to show an error.
| self.notify_service_id = app.config["NOTIFY_SERVICE_ID"] | ||
|
|
||
| def find_ids(self, ids): | ||
| data = self.get("/support/find-ids", params={"ids": ids}) |
There was a problem hiding this comment.
Consider adding error handling for the self.get method to manage potential exceptions or unsuccessful responses.
There was a problem hiding this comment.
nah, just show the error in the web app.
| _data={"search": "1234"}, | ||
| _expected_status=200, | ||
| ) | ||
| get_records.assert_called_once_with("1234") |
There was a problem hiding this comment.
Consider using a more specific assertion for the number of search inputs to ensure the test is more robust.
| _expected_status=200, | ||
| ) | ||
| get_records.assert_called_once_with("1234") | ||
| results = document.findAll("a", {"class": "browse-list-link"}) |
There was a problem hiding this comment.
Consider using find_all instead of findAll for consistency with modern BeautifulSoup conventions.
| _expected_status=200, | ||
| ) | ||
| get_records.assert_called_once_with("1234") | ||
| results = document.findAll("a", {"class": "browse-list-link"}) |
There was a problem hiding this comment.
Consider using find_all instead of findAll for consistency with modern BeautifulSoup conventions.
| _expected_status=200, | ||
| ) | ||
| get_records.assert_called_once_with("1234") | ||
| results = document.findAll("a", {"class": "browse-list-link"}) |
There was a problem hiding this comment.
Consider using find_all instead of findAll for consistency with modern BeautifulSoup conventions.
| ) | ||
| document = client_request.post("main.find_ids", _data={"search": "1234,5678"}, _expected_status=200) | ||
| get_records.assert_called_once_with("1234,5678") | ||
| results = document.findAll("a", {"class": "browse-list-link"}) |
There was a problem hiding this comment.
Consider using find_all instead of findAll for consistency with modern BeautifulSoup conventions.
Summary | Résumé
Add a page to allow queries of arbitrary uuids (notifications/services/templates/users/jobs) and get the relevant links.
Test instructions | Instructions pour tester la modification