Y24-368-392 BGE blending #2318
Y24-368-392 BGE blending #2318
Conversation
…e for tubes to tubes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2318 +/- ##
===========================================
+ Coverage 81.07% 81.46% +0.38%
===========================================
Files 476 480 +4
Lines 18231 18777 +546
Branches 269 353 +84
===========================================
+ Hits 14781 15296 +515
- Misses 3448 3479 +31
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -93,10 +94,14 @@ const elements = [ | |||
| userIdRequired: true, | |||
| }, | |||
| { | |||
| id: 'pool-xp-tube-submit-panel', | |||
| component: PoolXPTubeSubmitPanel, | |||
| id: 'blended-tube-page', | |||
There was a problem hiding this comment.
Duplicate registration for element id 'blended-tube-page' detected in vue_app.js. Consider removing the duplicate to prevent potential rendering conflicts.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
There was a problem hiding this comment.
Yes, looks like blended-tube-page is registered twice here.
| } else { | ||
| this.$set(this, 'isPairingValid', false), this.$set(this, 'parentTubes', []) | ||
| } | ||
| this.valid |
There was a problem hiding this comment.
Can this line be removed?
| this.progressMessage = 'Creating blended tube...' | ||
| this.loading = true | ||
|
|
||
| console.log('this.parentTubes:') |
There was a problem hiding this comment.
Can remove these console logs
| }, | ||
| methods: { | ||
| updateTubePair(data) { | ||
| if (data.state === 'valid') { |
There was a problem hiding this comment.
I think you can remove the use of this.$set in this method, I don't think it should be needed, e.g. can do this.isPairingValid = true should be enough for Vue here.
| return sampleIds | ||
| }, | ||
| validatedTubes() { | ||
| if (!this.areBothTubesScanned) { |
There was a problem hiding this comment.
This function can be simplified to return this.areBothTubesScanned && this.areBothTubesValid
|
|
||
| const ancestors = wrapper.vm.findAncestors() | ||
|
|
||
| expect(ancestors).toEqual([ |
There was a problem hiding this comment.
Should there be an error message check here too?
| </div> | ||
| </b-row> | ||
| </b-container> | ||
| <b-card bg-variant="white" text-variant="black" :header="headerSummary" header-tag="h5"> |
There was a problem hiding this comment.
I think you can change header here to header="Summary for this pairing:" and remove the function on line 217 as it doesn't need to be computed because its value should never change.
There was a problem hiding this comment.
Nice change, definitely better to be specific 👍
| @@ -93,10 +94,14 @@ const elements = [ | |||
| userIdRequired: true, | |||
| }, | |||
| { | |||
| id: 'pool-xp-tube-submit-panel', | |||
| component: PoolXPTubeSubmitPanel, | |||
| id: 'blended-tube-page', | |||
There was a problem hiding this comment.
Yes, looks like blended-tube-page is registered twice here.
| @@ -133,7 +133,7 @@ def expect_specific_tube_creation | |||
| specific_tubes_attributes.map do |attrs| | |||
| { | |||
| child_purpose_uuids: [attrs[:uuid]] * attrs[:child_tubes].size, | |||
| parent_uuids: [parent_uuid], | |||
| parent_uuids: attrs[:parent_uuids], | |||
There was a problem hiding this comment.
Guessing this was broken before?
Closes ##1981, #1997
Changes proposed in this pull request
Adds BGE Blending step