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

feat(ontology): check if an ontology, a class or a property can be deleted (DSP-1750) #457

Merged
merged 22 commits into from Jun 22, 2021

Conversation

@kilchenmann
Copy link
Collaborator

@kilchenmann kilchenmann commented Jun 7, 2021

resolves DSP-1700
is required for DSP-1623

In this PR I added the new DSP-API routes (DSP-1622) and DSP-JS methods (DSP-1673) to check if an ontology, a resource class or a property can be deleted or not.

@kilchenmann kilchenmann self-assigned this Jun 7, 2021
@kilchenmann kilchenmann marked this pull request as draft Jun 7, 2021
@kilchenmann kilchenmann force-pushed the wip/dsp-1623-check-edit-properties branch from daf3951 to 6c87033 Jun 17, 2021
@kilchenmann kilchenmann changed the title feat(ontology): only allow properties to be edited in the ontology editor which are not used feat(ontology): check if an ontology, a class or a property can be deleted (DSP-1750) Jun 19, 2021
@kilchenmann kilchenmann requested review from mdelez and waychal Jun 19, 2021
@kilchenmann kilchenmann marked this pull request as ready for review Jun 19, 2021
@@ -52,7 +52,7 @@
<mat-icon>tune</mat-icon>
</button>
<span
[matTooltip]="(resClasses.length > 0 ? 'The property can\'t be removed because it\'s in use' : 'Remove property from resource class')">
[matTooltip]="((resClasses.length > 0 || !propCanBeDeleted) ? 'The property can\'t be removed because it\'s in use' : 'Remove property from resource class')">
Copy link
Contributor

@mdelez mdelez Jun 21, 2021

Choose a reason for hiding this comment

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

I think this logic needs to be refactored. I can delete the property even though it says I can't. AS a side note, I think the delete icon should be the trash can for consistency reasons. The icon currently being used is normally used for "backspace".

Screen Shot 2021-06-21 at 11 04 00

Loading

Copy link
Collaborator Author

@kilchenmann kilchenmann Jun 22, 2021

Choose a reason for hiding this comment

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

resolved in 5db5353

Loading

Copy link
Contributor

@waychal waychal left a comment

It is same with me. I am able to delete the property even if it says that This property can't be removed because it's in use. I would suggest to disable the delete icon if the property / class can't be deleted.
I am also able to to delete classes. Can you give example in which case I can not delete the class?

Loading

@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 22, 2021

@waychal and @mdelez Thanks for the report. Yes, there's something wrong with the logic. In this case the tooltip needs a different text, because here we want to remove (this is why I don't use the trash-bin icon) the property from the resource class; it's not deleting the property. This part has to be done in the properties section / .../property route itself. I'm using in both view the same component, but the functionality is different. I will fix it

Loading

@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 22, 2021

I figured out what happened: I added the propCanBeDeleted condition to the wrong place. Resolved in 5db5353

The "remove property from resource class" will be implemented / fixed in the next iteration...

Loading

@kilchenmann kilchenmann requested review from mdelez and waychal Jun 22, 2021
mdelez
mdelez approved these changes Jun 22, 2021
@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 22, 2021

Thank you @mdelez

Loading

@kilchenmann kilchenmann merged commit fb0c275 into main Jun 22, 2021
8 checks passed
Loading
@kilchenmann kilchenmann deleted the wip/dsp-1623-check-edit-properties branch Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants