Mav 2831 #837
Mav 2831 #837
Conversation
mavis/test/data/file_generator.py
Outdated
| df[col] = df[col].map(replace_substrings) | ||
| return df | ||
|
|
||
| def replace_placeholders_in_text( |
There was a problem hiding this comment.
This method is not used. If we want to keep it, I think it'd make more sense in file_utils.py instead
mavis/test/fixtures/helpers.py
Outdated
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def sidekiq_helper(): |
There was a problem hiding this comment.
This fixture isn't used and doesn't need to exist
There was a problem hiding this comment.
Moved to the test file
| ) | ||
| response.raise_for_status() | ||
|
|
||
| success = response.status_code in [200, 201] |
There was a problem hiding this comment.
There's no need to check for success after response.raise_for_status(). This method should also not return anything, as the output is never used
There was a problem hiding this comment.
This was probably here earlier. Removed now.
| # Apply replacements using FileGenerator pattern | ||
| payload_content = template_content | ||
| for placeholder, value in replacements.items(): | ||
| payload_content = payload_content.replace(placeholder, value) |
There was a problem hiding this comment.
Could use the replace_placeholders_in_text method (currently in file_generator.py, but probably should be in file_utils.py)
There was a problem hiding this comment.
This does not work on test data files
mavis/test/helpers/sidekiq_helper.py
Outdated
| response.raise_for_status() | ||
| return response.json() if response.content else {} | ||
|
|
||
| def get_job_status(self, job_name: str) -> dict[str, Any]: |
There was a problem hiding this comment.
None of get_job_status, list_recurring_jobs or get_sidekiq_stats are used. These may be useful but I don't think it's worth keeping them
mavis/test/constants.py
Outdated
| def target_disease_code(self) -> str: | ||
| """Get the SNOMED code for the target disease of this vaccine.""" | ||
| target_disease_mapping = { | ||
| # Flu vaccines target influenza |
There was a problem hiding this comment.
There are a lot of comments. I assume they're coming from an AI? I don't mind them but some are quite unnecessary. I would remove these for example.
There was a problem hiding this comment.
Removed comments
| "<<VACCINATION_TIME>>": vaccination_time.strftime( | ||
| "%Y-%m-%dT%H:%M:%S+00:00" | ||
| ), | ||
| "<<RECORDED_TIME>>": datetime.now(tz=UTC).strftime( |
There was a problem hiding this comment.
Could consider using the get_current_datetime utility method in mavis.test.utils
mavis/test/helpers/sidekiq_helper.py
Outdated
| "initial_failed": initial_failed, | ||
| }, | ||
| "message": f"Recurring job '{job_name}' completed", | ||
| } |
There was a problem hiding this comment.
I'm not sure what the purpose of returning this dict is. If we want to keep it, we could remove some unnecessary fields ( e.g. "status": "completed" is not useful, we know the job completed already as no exception was thrown by the method)
mavis/test/fixtures/__init__.py
Outdated
| schedule_session_and_get_consent_url, | ||
| set_feature_flags, | ||
| setup_session_and_batches_with_fixed_child, | ||
| sidekiq_helper, |
There was a problem hiding this comment.
Looks good, think we just need to remove the sidekiq_helper mentions from this file
2 participants
Added test for syncing vaccinations from Imms to Mavis.