Skip to content

Sample tidy up #1035

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.

Sign up for GitHub

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

Open
LJBabbage wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Sample tidy up #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4fc5205
fix sample load time formatting
matthew-robinson-ons Oct 17, 2025
08dfac5
slight refactor
matthew-robinson-ons Oct 17, 2025
5653589
improve naming
matthew-robinson-ons Oct 17, 2025
f08a7f9
refactor test
matthew-robinson-ons Oct 17, 2025
d0fdfad
linter
matthew-robinson-ons Oct 17, 2025
6dc1959
Linter!
matthew-robinson-ons Oct 17, 2025
891ff4d
auto patch increment
ras-rm-pr-bot Oct 17, 2025
9a30a7c
different approach
matthew-robinson-ons Oct 17, 2025
0a10150
better varibale names
matthew-robinson-ons Oct 17, 2025
ec842ca
Tidy up sample section
LJBabbage Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions _infra/helm/response-operations-ui/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ type: application

# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version
version: 3.1.144
version: 3.1.145

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application.
appVersion: 3.1.144
appVersion: 3.1.145
2 changes: 1 addition & 1 deletion response_operations_ui/templates/collection_exercise/ce-sample-section.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ <h2 class="ons-u-fs-r--b ons-u-pt-m">Sample file *</h2>
{
"tds": [
{
"value": "Sample loaded: " + '<em>' + sample.ingestDateTime[:-7] + '</em>' + sample.ingestDateTime[-7:]
"value": "Sample loaded: " + '<em>' + sample_ingest_date_time[:-7] + '</em>' + sample_ingest_date_time[-7:]
},
{
"value": addSampleLink,
Expand Down
35 changes: 18 additions & 17 deletions response_operations_ui/views/collection_exercise.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,21 +107,23 @@ def _build_collection_instruments_details(collection_exercise_id: str, survey_id
@collection_exercise_bp.route("/<short_name>/<period>", methods=["GET"])
@login_required
def view_collection_exercise(short_name, period):
sample_load_status = None
sample_ingest_date_time =None
collection_exercise, survey = get_collection_exercise_and_survey_details(short_name, period)
sample = get_sample_summary(collection_exercise["id"])
events = convert_events_to_new_format(
collection_exercise_controllers.get_collection_exercise_events_by_id(collection_exercise["id"])
)
collection_instruments = _build_collection_instruments_details(collection_exercise["id"], survey["id"])
sample_load_status = None
if sample_controllers.sample_summary_state_check_required(collection_exercise["state"], sample):
try:
sample_load_status = sample_controllers.check_if_all_sample_units_present_for_sample_summary(sample["id"])
if sample_load_status["areAllSampleUnitsLoaded"]:
sample = _format_sample_summary(sample)

except ApiError:
flash("Sample summary check failed. Refresh page to try again", category="error")

if sample:

@LJBabbage LJBabbage Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

House keeping, we don't want to do any sample work if we don't have a sample

if sample_controllers.sample_summary_state_check_required(collection_exercise["state"], sample):
try:
sample_load_status = sample_controllers.check_if_all_sample_units_present_for_sample_summary(sample["id"])

@LJBabbage LJBabbage Oct 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very badly named function, it actually updates the sample "state" to ACTIVE if the count is correct

@LJBabbage LJBabbage Oct 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically you submit a sample, and it's using this to update whether it is ready. The first pass will always be INIT and you get the loading sample page on the UI. The refresh/revisiting is used to see if it can transition. This isn't the best design unfortunately

except ApiError:
flash("Sample summary check failed. Refresh page to try again", category="error")
if sample["state"] in ["ACTIVE", "COMPLETE"]:

@LJBabbage LJBabbage Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are only interested in states Active and complete (see if in ce-sample-section.html), so we can use that as a way of deciding if we want to format the ingestion date/time

@LJBabbage LJBabbage Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the state of the sample is anything but Active/complete, it will not show the bit where the date is shown

sample_ingest_date_time = _format_ingestion_date(sample.get("ingestDateTime"))

ce_state = collection_exercise["state"]
survey_mode = survey["surveyMode"]
Expand Down Expand Up @@ -163,6 +165,7 @@ def view_collection_exercise(short_name, period):
processing=processing,
sample_load_status=sample_load_status,
sample=sample,
sample_ingest_date_time=sample_ingest_date_time,
show_set_live_button=show_set_live_button,
survey=survey,
success_panel=success_panel,
Expand Down Expand Up @@ -465,14 +468,12 @@ def _validate_sample() -> bool:
return True


def _format_sample_summary(sample):
if sample and sample.get("ingestDateTime"):
submission_datetime = localise_datetime(iso8601.parse_date(sample["ingestDateTime"]))
submission_time = submission_datetime.strftime("%d %B %Y %I:%M%p")
sample["ingestDateTime"] = submission_time

@LJBabbage LJBabbage Oct 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to mess around with the sample, there is no need to pass it, then do checks, we only want the date and we know when to format it. We also don't want to mutate the sample, so we can just store and pass a separate var


return sample

def _format_ingestion_date(date_time):
try:
localised_submission_datetime = localise_datetime(iso8601.parse_date(date_time))
return localised_submission_datetime.strftime("%d %B %Y %I:%M%p")
except ValueError:
return date_time

def _format_ci_file_name(collection_instruments, survey_details):
for ci in collection_instruments:
Expand Down
Loading