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

OP-1043 | Implement soft deletion #2077

Conversation

SteveGT96
Copy link
Contributor

@SteveGT96 SteveGT96 commented Oct 30, 2024

See OP-1043

mwithi
mwithi previously approved these changes Nov 5, 2024
Copy link
Member

@mwithi mwithi left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot!

@mwithi mwithi requested a review from dbmalkovsky November 5, 2024 14:50
@mwithi mwithi dismissed their stale review November 5, 2024 18:28

found more issues

Copy link
Member

@mwithi mwithi left a comment

Choose a reason for hiding this comment

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

Although when editing a soft-deleted user or group it blocks the operation because the record "is already marked as deleted", I think we can live with it because why one should edit other fields on an already soft-deleted record? If really needed one can temporarly remove the "deleted" check, change the required fields and mark it again as deleted afterward.

Wdyt?

@SteveGT96
Copy link
Contributor Author

Although when editing a soft-deleted user or group it blocks the operation because the record "is already marked as deleted", I think we can live with it because why one should edit other fields on an already soft-deleted record? If really needed one can temporarly remove the "deleted" check, change the required fields and mark it again as deleted afterward.

Wdyt?

My Opinion at the beginning is that the edit boutton shoudn't be enabled after soft deletion. Instead, we should have just added a "Restore" button that is only shown when the selected item is soft deleted(by clicking on it, we restore the item and therefore make the item editable again).

@SteveGT96
Copy link
Contributor Author

@mwithi, the pipeline is broken.

@mwithi
Copy link
Member

mwithi commented Nov 12, 2024

@mwithi, the pipeline is broken.

it's fine, it's because it is taking informatici core instead of your core, IDKW.... on a next commit from your should be ok

@mwithi
Copy link
Member

mwithi commented Nov 12, 2024

My Opinion at the beginning is that the edit boutton shoudn't be enabled after soft deletion. Instead, we should have just added a "Restore" button that is only shown when the selected item is soft deleted(by clicking on it, we restore the item and therefore make the item editable again).

If you want to implement like that is ok also, but in GUI we never had something like "Restore" and we mostly stick to "Edit" term for any operation... we will update the DOC though, so well explained everything is possible.

@SteveGT96
Copy link
Contributor Author

My Opinion at the beginning is that the edit boutton shoudn't be enabled after soft deletion. Instead, we should have just added a "Restore" button that is only shown when the selected item is soft deleted(by clicking on it, we restore the item and therefore make the item editable again).

If you want to implement like that is ok also, but in GUI we never had something like "Restore" and we mostly stick to "Edit" term for any operation... we will update the DOC though, so well explained everything is possible.

We could do that later in an enhancement task.

@mwithi
Copy link
Member

mwithi commented Nov 13, 2024

I think checks are failing because GUI and CORE are not equally aligned with develop

@mwithi mwithi merged commit f4b07db into informatici:develop Nov 14, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants