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
[CLA Needed] Addresses #6593 #7684
base: master
Are you sure you want to change the base?
Conversation
fix: valid indicators not updating properly for Gauge plugin limits refactor: enforce limitLow to be less than or equal to limitHigh
fix: OK button not properly disabling when data is invalid note: (implementation works around the broken validation mechanism)
const validate = data.model.validate; | ||
if (valid && validate) { | ||
valid = validate(data); | ||
if (data.isValid !== undefined && data.isValid !== null && !data.isValid) { |
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 created an 'isValid' property on 'data' to work around the broken 'data.model.validate' logic below.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7684 +/- ##
==========================================
- Coverage 56.62% 56.31% -0.32%
==========================================
Files 672 672
Lines 27124 27125 +1
Branches 2632 2634 +2
==========================================
- Hits 15359 15275 -84
- Misses 11437 11521 +84
- Partials 328 329 +1
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
I need assistance completing the unchecked 'Author Checklist':
|
Hi @TheFuturist32, Awesome, thanks for taking a look at this! If you haven't already, please complete and submit a Contributor License Agreement (CLA). This is required before we can merge your code.
The short answer is yes-- the question is what kind of tests would best cover these changes? I think an e2e test which does the following would be sufficient:
We use Playwright for our e2e testing framework. To learn more about our e2e framework and testing strategies, check out our e2e docs. Let us know if we there's anything we can do to help you get started.
We can take care of these for you 😎 Thanks again for your contribution and let us know if you have any questions! |
FYSA: I have not abandoned this PR. "there is a bit of work required to get from 0 to 1 test contributed", there's certainly a learning curve and I am working on it. In fact, I am not experienced with JS/Vue so the entire project is a learning curve, which is why I chose this project to support. I appreciate the opportunity to learn. Side note: While looking through all of the documentation around contributing and testing, I found some broken links and typos. Is that something I can add to this PR, or should I create a separate PR to address those? |
Sure, feel free to fix any typos as a part of this PR. Let us know if you have any questions |
Closes #6593
Describe your changes:
All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist