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

Fix all code that is using creatability as a proxy for persistability #4323

Closed
1 of 5 tasks
akhenry opened this issue Oct 15, 2021 · 3 comments · Fixed by #4898
Closed
1 of 5 tasks

Fix all code that is using creatability as a proxy for persistability #4323

akhenry opened this issue Oct 15, 2021 · 3 comments · Fixed by #4898

Comments

@akhenry
Copy link
Contributor

akhenry commented Oct 15, 2021

Summary

Some of our code is using the creatable property of object types to determine whether an object can be persisted. Creatability and persistability are two different things. The creatable property only tells you whether or not an object type appears in the Create menu and nothing else. An overlay plot can be created from the Create menu, but it can also be exposed as static JSON in the tree. Using creatability to determine whether the object can be persisted in this case will give a false impression, and will result in persistence errors if any mutation or persistence is attempted.

We need to search our code base for instances where the .creatable attribute is being used to determine persistability, and replace it with a call to openmct.object.isPersistable(domainObject).

Here is an example of an instance where creatability is being used instead of persistability:
#4149 (comment)

This is of fairly marginal impact right now, but it will soon become critical when we start building change-controlled display layouts and exposing them as non-editable objects in the tree.

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?
@jvigliotta
Copy link
Contributor

Testing

  • make sure you can't select dictionary items/folders as destinations from the create menu
  • make sure Duplicate, Move, Link, Remove, ExportAsJSON, ImportAsJSON actions are working properly
  • try to edit or modify dictionary items or other non-editable items (Plans folder...)

@nikhilmandlik
Copy link
Contributor

able edit properties for plans inside a Viper Plans and use another plan json.

@unlikelyzero unlikelyzero self-assigned this Mar 14, 2022
@unlikelyzero unlikelyzero added needs:e2e Needs an e2e test and removed unverified labels Mar 14, 2022
@unlikelyzero unlikelyzero added this to To triage in Improve Test Coverage via automation Mar 14, 2022
@jvigliotta jvigliotta mentioned this issue Mar 16, 2022
15 tasks
@jvigliotta jvigliotta removed the needs:test instructions Missing testing notes label Mar 17, 2022
@unlikelyzero
Copy link
Collaborator

unlikelyzero commented May 4, 2022

@unlikelyzero unlikelyzero moved this from To triage to Done in Improve Test Coverage May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

5 participants