Skip to content
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

SIMSBIOHUB-345: Display Time in Sampling Periods #1167

Merged
merged 8 commits into from
Nov 8, 2023

Conversation

KjartanE
Copy link
Contributor

@KjartanE KjartanE commented Nov 7, 2023

Links to Jira Tickets

Description of Changes

  • Added Time picker for start and end time on sample site periods.
  • Database changes include 2 more columns for start,end time.

Testing Notes

  • {List any relevant testing considerations, necessary pre-reqs, and areas of the app to focus on. Specifically, include anything that will help the reviewers of this PR verify the code is functioning as expected.}

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #1167 (f91231d) into dev-staging (ec677ac) will decrease coverage by 0.18%.
Report is 2 commits behind head on dev-staging.
The diff coverage is 0.00%.

@@               Coverage Diff               @@
##           dev-staging    #1167      +/-   ##
===============================================
- Coverage        61.49%   61.32%   -0.18%     
===============================================
  Files              551      552       +1     
  Lines            16657    16702      +45     
  Branches          2551     2570      +19     
===============================================
- Hits             10244    10243       -1     
- Misses            5741     5782      +41     
- Partials           672      677       +5     
Files Coverage Δ
...{projectId}/survey/{surveyId}/sample-site/index.ts 89.36% <ø> (ø)
...urveyId}/sample-site/{surveySampleSiteId}/index.ts 87.71% <ø> (ø)
api/src/services/sample-method-service.ts 97.22% <ø> (ø)
...features/surveys/components/EditSamplingMethod.tsx 0.00% <ø> (ø)
...s/observations/sampling-sites/SamplingSiteList.tsx 0.00% <0.00%> (ø)
...ervations/observations-table/ObservationsTable.tsx 0.00% <0.00%> (ø)
api/src/repositories/sample-period-repository.ts 68.57% <0.00%> (-8.85%) ⬇️
app/src/features/surveys/components/MethodForm.tsx 0.00% <0.00%> (ø)
...features/surveys/components/SamplingMethodForm.tsx 0.00% <0.00%> (ø)
...ing-sites/edit/components/SampleMethodEditForm.tsx 0.00% <0.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@GrahamS-Quartech
Copy link
Contributor

Couple things I noticed:

  • The modal will allow you to set an end time occurring before the start time when the dates are the same.
  • If you enter the edit sampling site page, then edit either time period time field and hit Update in that modal, it will update the card with that new time. However if you re-enter that edit modal, the time will have reverted to the previous value.
  • I think the layout of how the fields and trash can are arranged could be better but I don't really know what would look best. Maybe check in with Jeremy?

Copy link
Contributor

@al-rosenthal al-rosenthal left a comment

Choose a reason for hiding this comment

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

adding and editing works as expected

app/src/features/surveys/components/MethodForm.tsx Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Nov 7, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
4.8% 4.8% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

Copy link
Contributor

@al-rosenthal al-rosenthal left a comment

Choose a reason for hiding this comment

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

🕐

Copy link
Contributor

@JeremyQuartech JeremyQuartech left a comment

Choose a reason for hiding this comment

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

Tested locally, everything working correctly

@al-rosenthal al-rosenthal merged commit a02df6f into dev-staging Nov 8, 2023
17 of 20 checks passed
@al-rosenthal al-rosenthal deleted the SIMSBIOHUB-345_PR branch November 8, 2023 17:05
<Icon path={mdiClockOutline} size={1} />
</ListItemIcon>
<ListItemText
primary={`${period.start_time} ${(period.end_time && `to ${period.end_time}`) || ''}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could avoid the trailing whitespace:

primary={[period.start_time, period.end_time].filter(Boolean).join(' to ')}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants