-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: 1435 different parent tree gw values and date update #1491
Conversation
…5-different-parent-tree-gw-values
…5-different-parent-tree-gw-values # Conflicts: # backend/src/main/java/ca/bc/gov/backendstartapi/service/TscAdminService.java # backend/src/test/java/ca/bc/gov/backendstartapi/repository/seedlot/SeedlotEntityRelationalTest.java # backend/src/test/java/ca/bc/gov/backendstartapi/repository/seedlot/SeedlotRelationalTest.java # backend/src/test/java/ca/bc/gov/backendstartapi/service/SeedlotCollectionMethodServiceTest.java # backend/src/test/java/ca/bc/gov/backendstartapi/service/SeedlotFormPutTest.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job @craigyu!
Added a few markups, but nothing major, I'll tag them as resolved so we can merge the PR as soon as possible, but you can take a look at them later.
backend/src/main/java/ca/bc/gov/backendstartapi/service/OrchardService.java
Show resolved
Hide resolved
backend/src/main/java/ca/bc/gov/backendstartapi/service/OrchardService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/ca/bc/gov/backendstartapi/service/SeedlotService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/ca/bc/gov/backendstartapi/service/SeedlotService.java
Outdated
Show resolved
Hide resolved
frontend/src/components/SeedlotRegistrationSteps/ParentTreeStep/TableComponents/index.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/SeedlotRegistrationSteps/ParentTreeStep/constants.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job Craig! Only a few items. 💯 🥇
backend/src/main/java/ca/bc/gov/backendstartapi/endpoint/GeneticWorthEndpoint.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/ca/bc/gov/backendstartapi/endpoint/OrchardEndpoint.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/ca/bc/gov/backendstartapi/entity/GeneticWorthEntity.java
Show resolved
Hide resolved
backend/src/main/java/ca/bc/gov/backendstartapi/entity/GeneticWorthEntity.java
Outdated
Show resolved
Hide resolved
oracle-api/src/main/java/ca/bc/gov/oracleapi/entity/projection/ParentTreeProj.java
Show resolved
Hide resolved
oracle-api/src/main/java/ca/bc/gov/oracleapi/service/ParentTreeService.java
Outdated
Show resolved
Hide resolved
oracle-api/src/main/java/ca/bc/gov/oracleapi/repository/ParentTreeRepository.java
Outdated
Show resolved
Hide resolved
frontend/src/utils/SeedlotUtils.ts
Outdated
createdAt: utcToLocalFormat(seedlot.auditInformation.entryTimestamp), | ||
lastUpdatedAt: utcToLocalFormat(seedlot.auditInformation.updateTimestamp), | ||
approvedAt: utcToLocalFormat(seedlot.approvedTimestamp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (non-blocking): I'm not sure we're actually displaying the time. I can see only dates here. That's the way it should be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall seeing time on Figma, i think adding time the column will be too wide in tables, let's consult @mmarsoleta later in the standup
…ithub.com/bcgov/nr-spar into feat/1435-different-parent-tree-gw-values
…5-different-parent-tree-gw-values # Conflicts: # frontend/src/components/SeedlotReviewSteps/ExtractionStorage/Read/index.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Completes the implementation part of #1435, tests needed
!!! Needs to wipe the draft table after merge !!!
ETL should be unaffected
I will fix and add tests on Monday, let's get changes in so we can test and spot issues. Currently, affected tests are commented out but marked with
TODO
.I will also remove the oracle-api endpoint
/api/orchards/parent-trees/vegetation-codes/{vegCode}
, the new endpoint is now/api/parent-trees/vegetation-codes/{vegCode}
Changelog
New
Changed
Removed
How was this tested?
What gif/image best describes this PR or how it makes you feel?
Thanks for the PR!
Deployments, as required, will be available below:
Please create PRs in draft mode. Mark as ready to enable:
After merge, new images are deployed in:
Thanks for the PR!
Deployments, as required, will be available below:
Please create PRs in draft mode. Mark as ready to enable:
After merge, new images are deployed in: