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.

Sign up for GitHub

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

Jump to bottom

[Gauge] Editing any single config property causes all other config properties to become undefined #5985

Closed
3 of 7 tasks
ozyx opened this issue Nov 15, 2022 · 2 comments
Closed
3 of 7 tasks

[Gauge] Editing any single config property causes all other config properties to become undefined #5985

ozyx opened this issue Nov 15, 2022 · 2 comments
Labels
severity:blocker type:bug
Milestone
Target:2.1.3

Comments

@ozyx
Copy link
Contributor

ozyx commented Nov 15, 2022

Summary

When editing config properties of a gauge, all other properties become undefined causing the Gauge to go into a bad state and break.

Expected vs Current Behavior

Configuration properties should not be lost, Gauge should not break on property edit

Steps to Reproduce

  1. Create a Gauge, save it
  2. Edit Properties of the Gauge
  3. Change any configuration option (anything that's not name or notes, Gauge specific properties)
  4. Click Save
  5. Observe a ton of errors in the console due to things being undefined

Environment

  • Open MCT Version: 2.1.4-SNAPSHOT
  • Deployment Type: local
  • OS: MacOS
  • Browser: Chrome/ any

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?

Additional Information
@ozyx ozyx added the type:bug label Nov 15, 2022
@ozyx ozyx added this to the Target:2.1.3 milestone Nov 15, 2022
@ozyx ozyx self-assigned this Nov 15, 2022
@ozyx ozyx mentioned this issue Nov 15, 2022
Closed
7 tasks
ozyx added a commit that referenced this issue Nov 15, 2022
@ozyx
fix(#5985): deep merge on create/edit properties
4aa4f2b 
- Perform a deep merge of old and new properties on Create/Edit properties actions
@ozyx ozyx mentioned this issue Nov 15, 2022
Merged
15 tasks
@ozyx
Copy link
Contributor Author

ozyx commented Nov 15, 2022

Testing Instructions

  1. Create a Gauge with defaults
  2. Save it to get out of edit mode
  3. Click on the 3dot menu, click "Edit Properties"
  4. Change a single Gauge-specific configuration option (anything that isnt name or notes)
  5. Save it
  6. Verify no console errors
  7. Click edit properties again
  8. Verify that the properties persisted and no config properties are blank

shefalijoshi pushed a commit that referenced this issue Nov 15, 2022
@ozyx
Preserve Gauge configuration changes on create/edit (#5986)
adce5ca 
* fix(#5985): deep merge on create/edit properties

- Perform a deep merge of old and new properties on Create/Edit properties actions

* refactor(e2e): improve selector in appActions

* test(e2e): add tests for gauges

- test creating a non-default gauge (checks only for console errors)
- test updating a gauge (checks only for console errors)

* fix(e2e): use pluginFixtures for gauge tests

* fix(e2e): prevent fail if testNotes is undefined
@jvigliotta
Copy link
Contributor

jvigliotta commented Nov 17, 2022 •
edited
Loading

Verified Fixed: Testathon 11/17/2022

jvigliotta pushed a commit that referenced this issue Nov 30, 2022
@ozyx @shefalijoshi
Merge release/2.1.3 into master (#6015)
40afb04 
* Bump version to `2.1.3` (#5973)
* Preserve Gauge configuration changes on create/edit (#5986)
* fix(#5985): deep merge on create/edit properties
- Perform a deep merge of old and new properties on Create/Edit properties actions
* refactor(e2e): improve selector in appActions
* test(e2e): add tests for gauges
- test creating a non-default gauge (checks only for console errors)
- test updating a gauge (checks only for console errors)
* fix(e2e): use pluginFixtures for gauge tests
* fix(e2e): prevent fail if testNotes is undefined
* Make the tree key unique (#5989)

Co-authored-by: Shefali Joshi <[email protected]>
shefalijoshi added a commit that referenced this issue Dec 12, 2022
@shefalijoshi @khalidadil @ozyx
Multiple y axes model changes for plots (#6026)
e76e85f 
* Update version for sprint 2.1.1

* Basic changes (very draft) for multiple y axis for plots

* Generalize additional axes model handling

* [Plot] Set yKey on each plot series (#5790)

* Set yKey on each model

* Add a test

* Update yaxis to be independent of mctplot

* Fix yAxis ticks

* Fix yAxisModel label

* Add multiple y axes option for overlay plot. UI changes to display 2 y-axes

* Fix Chart bug

* Bump version to `2.1.3` (#5973)

* Preserve Gauge configuration changes on create/edit (#5986)

* fix(#5985): deep merge on create/edit properties

- Perform a deep merge of old and new properties on Create/Edit properties actions

* refactor(e2e): improve selector in appActions

* test(e2e): add tests for gauges

- test creating a non-default gauge (checks only for console errors)
- test updating a gauge (checks only for console errors)

* fix(e2e): use pluginFixtures for gauge tests

* fix(e2e): prevent fail if testNotes is undefined

* Make the tree key unique (#5989)

* Refactor iterating over all yAxis while drawing to the viewport

* Refactor code to remove hardcoding of yAxisIds

* style: auto-fix lint errors

* Remove configurability of number of y axes. Use yAxisIds from Series to show/hide axes.
Adds test for multiple y axis display

* Add more tests for when only 1 axis needs to be displayed

* Address code review comments:
Move styles to computed properties
Refactor yAxes rendering in MctPlot to also include main y axis.

* Address review comments. Refactor plot configuration code for additiona y axes and reset the chart only for series that belong to the yaxis that has changed.

* Fix bug where only the first series for a given yAxis was rendering

* Refactor code to reuse method logic, move const and use cap case

* Only draw highlights for the given yAxis

* Only draw rectangles for the yAxis the series belongs to

Co-authored-by: Khalid Adil <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
ozyx added a commit that referenced this issue Dec 28, 2022
@shefalijoshi @khalidadil @ozyx
Multiple y axes model changes for plots (#6026)
cfda067 
* Update version for sprint 2.1.1

* Basic changes (very draft) for multiple y axis for plots

* Generalize additional axes model handling

* [Plot] Set yKey on each plot series (#5790)

* Set yKey on each model

* Add a test

* Update yaxis to be independent of mctplot

* Fix yAxis ticks

* Fix yAxisModel label

* Add multiple y axes option for overlay plot. UI changes to display 2 y-axes

* Fix Chart bug

* Bump version to `2.1.3` (#5973)

* Preserve Gauge configuration changes on create/edit (#5986)

* fix(#5985): deep merge on create/edit properties

- Perform a deep merge of old and new properties on Create/Edit properties actions

* refactor(e2e): improve selector in appActions

* test(e2e): add tests for gauges

- test creating a non-default gauge (checks only for console errors)
- test updating a gauge (checks only for console errors)

* fix(e2e): use pluginFixtures for gauge tests

* fix(e2e): prevent fail if testNotes is undefined

* Make the tree key unique (#5989)

* Refactor iterating over all yAxis while drawing to the viewport

* Refactor code to remove hardcoding of yAxisIds

* style: auto-fix lint errors

* Remove configurability of number of y axes. Use yAxisIds from Series to show/hide axes.
Adds test for multiple y axis display

* Add more tests for when only 1 axis needs to be displayed

* Address code review comments:
Move styles to computed properties
Refactor yAxes rendering in MctPlot to also include main y axis.

* Address review comments. Refactor plot configuration code for additiona y axes and reset the chart only for series that belong to the yaxis that has changed.

* Fix bug where only the first series for a given yAxis was rendering

* Refactor code to reuse method logic, move const and use cap case

* Only draw highlights for the given yAxis

* Only draw rectangles for the yAxis the series belongs to

Co-authored-by: Khalid Adil <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
ozyx added a commit that referenced this issue Jan 20, 2023
@shefalijoshi @khalidadil @ozyx
Multiple y axes model changes for plots (#6026)
d4d6d66 
* Update version for sprint 2.1.1

* Basic changes (very draft) for multiple y axis for plots

* Generalize additional axes model handling

* [Plot] Set yKey on each plot series (#5790)

* Set yKey on each model

* Add a test

* Update yaxis to be independent of mctplot

* Fix yAxis ticks

* Fix yAxisModel label

* Add multiple y axes option for overlay plot. UI changes to display 2 y-axes

* Fix Chart bug

* Bump version to `2.1.3` (#5973)

* Preserve Gauge configuration changes on create/edit (#5986)

* fix(#5985): deep merge on create/edit properties

- Perform a deep merge of old and new properties on Create/Edit properties actions

* refactor(e2e): improve selector in appActions

* test(e2e): add tests for gauges

- test creating a non-default gauge (checks only for console errors)
- test updating a gauge (checks only for console errors)

* fix(e2e): use pluginFixtures for gauge tests

* fix(e2e): prevent fail if testNotes is undefined

* Make the tree key unique (#5989)

* Refactor iterating over all yAxis while drawing to the viewport

* Refactor code to remove hardcoding of yAxisIds

* style: auto-fix lint errors

* Remove configurability of number of y axes. Use yAxisIds from Series to show/hide axes.
Adds test for multiple y axis display

* Add more tests for when only 1 axis needs to be displayed

* Address code review comments:
Move styles to computed properties
Refactor yAxes rendering in MctPlot to also include main y axis.

* Address review comments. Refactor plot configuration code for additiona y axes and reset the chart only for series that belong to the yaxis that has changed.

* Fix bug where only the first series for a given yAxis was rendering

* Refactor code to reuse method logic, move const and use cap case

* Only draw highlights for the given yAxis

* Only draw rectangles for the yAxis the series belongs to

Co-authored-by: Khalid Adil <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
severity:blocker type:bug
Projects
None yet
Milestone
Target:2.1.3
Development

No branches or pull requests
4 participants
@shefalijoshi @ozyx @jvigliotta @unlikelyzero