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

UI: Bugs around dots in task/task-group/driver names #4994

Merged
merged 6 commits into from
Dec 17, 2018

Conversation

DingoEatingFuzz
Copy link
Contributor

There is a repeated pattern in serializers in the UI code of using Ember.get to read properties off of potentially undefined objects. Unfortunately, Ember.get doesn't perfectly match plain JavaScript in its allowance of property names: dots in Ember.get paths are always treated as nesting, meanwhile, dots are valid characters in properties in JavaScript and therefore JSON.

This PR adds a bunch of unit tests to the offending serializers to expose the bugs and then squashes all the bugs by removing this pattern.

  1. Allocation: task names with dots caused a "cannot read property from an undefined object", resulting in an error page
  2. Deployment: task groups with dots would silently not have the correct properties.
  3. Evaluation: failed task groups with dots would silently not have the correct properties.
  4. JobPlan: same as evaluations.
  5. JobSummary: task groups with dots would silently not have allocation status properties.
  6. Node: drivers with dots (there are none, but there could be in the future) would silently not have any properties, such as detected or healthy.

'model:allocation',
'model:job',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll be glad to know new testing doesn't have needs 🎉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't wait.

Copy link
Contributor

@meirish meirish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've had to work around this a few times as well 😬 - very straightforward implementation here though and 👍 nice table tests ✨.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants