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-466: Add Observation Measurement Columns #1213

Merged
merged 109 commits into from
Mar 1, 2024
Merged

Conversation

NickPhura
Copy link
Collaborator

@NickPhura NickPhura commented Feb 14, 2024

Links to Jira Tickets

SIMSBIOHUB-466

Description of Changes

Many updates to the Observation Table components and related components

  • Support for adding, removing, hiding, and populating measurement columns.
  • New database tables + related service/repo

When creating an observation record, a new subcount record is also created.

Addressed a handful of dependency array warnings and "update a component while rendering another" warnings

Some misc package updates + related fixes.

Pending Critterbase updates

Wire up the stubbed calls to Critterbase, once its new endpoints are ready.

TODOs for another ticket

Update the telemetry datagrid components/tables to use the latest updates from the survey observations components.

Fix the 3-4 app tests that started to fail (probably a side effect of package updates?) I tried fixed them but was making no progress, so they are skipped for now. They are all related to trying to capture select box clicks, which is historically tricky.

Testing Notes

Observations table works as expected, when adding and removing observations with and without measurement columns.

NickPhura and others added 30 commits February 2, 2024 17:54
- Manage column components (initial structure)
- Autocomplete with stubbed response (issue: input clearing on option select)
- Not wired up to sims backend yet
- Not wired up to observations table yet
@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 33.71429% with 116 lines in your changes are missing coverage. Please review.

Project coverage is 76.02%. Comparing base (a7cdac3) to head (1e11eda).

Files Patch % Lines
api/src/middleware/critterbase-proxy.ts 0.00% 28 Missing ⚠️
api/src/services/observation-service.ts 22.85% 27 Missing ⚠️
api/src/repositories/subcount-repository.ts 25.00% 24 Missing ⚠️
api/src/services/subcount-service.ts 26.66% 11 Missing ⚠️
api/src/services/platform-service.ts 0.00% 8 Missing ⚠️
api/src/database/db.ts 25.00% 6 Missing ⚠️
api/src/app.ts 0.00% 5 Missing ⚠️
api/src/services/critterbase-service.ts 66.66% 5 Missing ⚠️
...rojectId}/survey/{surveyId}/observations/delete.ts 0.00% 1 Missing ⚠️
api/src/services/bctw-service.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##              dev    #1213       +/-   ##
===========================================
+ Coverage   56.26%   76.02%   +19.75%     
===========================================
  Files         576      235      -341     
  Lines       17647     9629     -8018     
  Branches     2751     1353     -1398     
===========================================
- Hits         9929     7320     -2609     
+ Misses       7056     1896     -5160     
+ Partials      662      413      -249     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NickPhura NickPhura added the Ready For Review PR is ready for review label Feb 26, 2024
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.

Ran into a few issues when testing this locally:
Validation Banner doesn't show

  • Hide required columns (like the sample sites)
  • Add new row and fill
  • Save (see error dialog)
  • No banner

Hiding newly added columns

  • Add new measurement
  • New column cannot be toggled
  • Close/ Reopen measurements panel
  • Newly added measurement is still cannot be toggled

Deleting Columns from measurement popup

  • Removing a single measurement from the list seems to work as expected
  • Removing all added measurements seems to reload the panel and add columns back to list (might be data related)

@NickPhura
Copy link
Collaborator Author

@al-rosenthal

Ran into a few issues when testing this locally: Validation Banner doesn't show

I think this is an existing issue. It seems that we only validate the sampling columns if at least one of them is populated, probably to do with the observation import maybe? The comments in that code talk about not validating sampling columns if its an incidental observation.

Hiding newly added columns

I fixed a few issues with the visibility controls. I think it should be happy now.

Deleting Columns from measurement popup

Yeah this is a more interesting issue. The problem is that the measurement data is still part of the row data, and just with the way the useEffect is set up right now, when you remove all measurement columns it re-loads the columns from the existing data, hence those columns showing up again suddenly. There should be a fix, but to date we only ever do row based deletes, so ill have to poke at it.

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.

Retested this and saw something else:

  • Add a new row
  • Discard Changes
  • Open Measurement Panel
  • Cannot add measurements (probably an issue with state)

@NickPhura
Copy link
Collaborator Author

@al-rosenthal

Retested this and saw something else:

Should be fixed. Was not resetting the row modes model on discard

curtisupshall
curtisupshall previously approved these changes Feb 29, 2024
Copy link
Contributor

@curtisupshall curtisupshall left a 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! 📏

al-rosenthal
al-rosenthal previously approved these changes Feb 29, 2024
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.

🍾

@NickPhura NickPhura dismissed stale reviews from al-rosenthal and curtisupshall via 63af389 February 29, 2024 18:39
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

Quality Gate Failed Quality Gate failed

Failed conditions
12.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@NickPhura NickPhura merged commit d2a1c85 into dev Mar 1, 2024
16 of 17 checks passed
@NickPhura NickPhura deleted the SIMSBIOHUB-466 branch March 1, 2024 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants