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(resource): delete and erase resource (DSP-1228) #489

Merged
merged 9 commits into from Jul 26, 2021

Conversation

kilchenmann
Copy link
Contributor

@kilchenmann kilchenmann commented Jul 20, 2021

resolves DSP-1228

What I did:

  • new section in dialog component for delete (with comment) and for erase (without comment)
  • integration of delete and erase methods from js-lib
  • after deleting it displays (deleted) behind the label
  • app-specific message banner to display info, that the resource has been deleted
  • if resource doesn't exist (anymore) the api returns a 404 error. In this case display text, that the resource does not exist or could not be found.
  • check if user is system or project admin; this information is needed to enable or disable the erase button.
  • not sure how to handle permissions: the edit menu is activated in case a user is logged-in

@kilchenmann kilchenmann self-assigned this Jul 20, 2021
@kilchenmann kilchenmann added dependencies Pull requests that update a dependency file enhancement New feature or request refactor Refactor or redesign labels Jul 20, 2021
@kilchenmann kilchenmann requested a review from mdelez July 22, 2021 06:06
Copy link
Collaborator

@mdelez mdelez left a comment

Choose a reason for hiding this comment

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

If I edit the value of a resource property, I can no longer delete or erase the resource. Seems to be an issue with the API:
Screen Shot 2021-07-22 at 11 27 34

@kilchenmann
Copy link
Contributor Author

kilchenmann commented Jul 22, 2021

If I edit the value of a resource property, I can no longer delete or erase the resource. Seems to be an issue with the API:

Thank you for this report. Yes, in this case we should take the last modification date into account (somehow).

@kilchenmann
Copy link
Contributor Author

kilchenmann commented Jul 23, 2021

@mdelez I fixed the issue you mentioned (#489 (review)) in commit ad54fa8. But it's not the best solution yet. I think we should emit the last modification date from ui-lib's property-value component. I wrote already a corresponding issue on youtrack: DSP-1813 --> not necessary anymore. DSP-UI returns already kind of lastModificationDate. Resolved in 6415484

@kilchenmann kilchenmann requested a review from mdelez July 23, 2021 09:00
@kilchenmann
Copy link
Contributor Author

kilchenmann commented Jul 23, 2021

@mdelez This PR should be ready for review again

Copy link
Collaborator

@mdelez mdelez left a comment

Choose a reason for hiding this comment

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

Nice :)

@kilchenmann kilchenmann merged commit 8b1fdba into main Jul 26, 2021
@kilchenmann kilchenmann deleted the wip/dsp-1228-delete-res-instance branch July 26, 2021 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request refactor Refactor or redesign
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants