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

Support recovery of deleted entities (fixes #88) #156

Merged
merged 7 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions backend/alembic/versions/5e46d4ff6f21_change_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
"""ChangeType for restoring deleted items

Revision ID: 5e46d4ff6f21
Revises: 8a3f36ffa8df
Create Date: 2023-09-25 15:28:13.752183

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = '5e46d4ff6f21'
down_revision = '8a3f36ffa8df'
branch_labels = None
depends_on = None


def upgrade():
op.execute("ALTER TYPE changetype ADD VALUE IF NOT EXISTS 'RESTORE'")


def downgrade():
pass
crazyscientist marked this conversation as resolved.
Show resolved Hide resolved
22 changes: 21 additions & 1 deletion backend/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,10 +645,13 @@ def update_entity(db: Session, id_or_slug: Union[str, int], schema_id: int, data
q = select(Entity).where(Entity.schema_id == schema_id)
q = q.where(Entity.id == id_or_slug) if isinstance(id_or_slug, int) else q.where(Entity.slug == id_or_slug)
e = db.execute(q).scalar()
if e is None or e.deleted:
if e is None:
raise MissingEntityException(obj_id=id_or_slug)
if e.schema.deleted:
raise MissingSchemaException(obj_id=e.schema.id)
if e.deleted:
# Updating a deleted entity implies its restoration.
e.deleted = False
der-gabe marked this conversation as resolved.
Show resolved Hide resolved

slug = data.pop('slug', e.slug)
name = data.pop('name', e.name)
Expand Down Expand Up @@ -730,3 +733,20 @@ def delete_entity(db: Session, id_or_slug: Union[int, str], schema_id: int, comm
else:
db.flush()
return e


def restore_entity(db: Session, id_or_slug: Union[int, str], schema_id: int, commit: bool = True) -> Entity:
q = select(Entity).where(Entity.schema_id == schema_id).where(Entity.deleted == True)
if isinstance(id_or_slug, int):
q = q.where(Entity.id == id_or_slug)
else:
q = q.where(Entity.slug == id_or_slug)
e = db.execute(q).scalar()
if e is None:
raise MissingEntityException(obj_id=id_or_slug)
e.deleted = False
if commit:
db.commit()
else:
db.flush()
return e
11 changes: 8 additions & 3 deletions backend/dynamic_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

from .traceability.entity import create_entity_create_request, create_entity_update_request, \
create_entity_delete_request, apply_entity_create_request, apply_entity_update_request, \
apply_entity_delete_request
apply_entity_delete_request, create_entity_restore_request, apply_entity_restore_request


factory = EntityModelFactory()
Expand Down Expand Up @@ -320,14 +320,19 @@ def route_delete_entity(router: APIRouter, schema: Schema):
}
)
def delete_entity(id_or_slug: Union[int, str], response: Response,
restore: bool = False,
db: Session = Depends(get_db),
user: User = Depends(req_permission)):
create_fun, apply_fun = create_entity_delete_request, apply_entity_delete_request
if restore:
create_fun, apply_fun = create_entity_restore_request, apply_entity_restore_request

try:
change_request = create_entity_delete_request(
change_request = create_fun(
db=db, id_or_slug=id_or_slug, schema_id=schema.id, created_by=user, commit=False
)
if not schema.reviewable:
return apply_entity_delete_request(
return apply_fun(
db=db, change_request=change_request, reviewed_by=user, comment='Autosubmit'
)[1]
db.commit()
Expand Down
5 changes: 5 additions & 0 deletions backend/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ def __str__(self) -> str:
return f'There is no entity delete request with id {self.obj_id}'


class MissingEntityRestoreRequestException(MissingObjectException):
def __str__(self) -> str:
return f'There is no entity restore request with id {self.obj_id}'


class MissingEntityCreateRequestException(MissingObjectException):
def __str__(self) -> str:
return f'There is no entity create request with id {self.obj_id}'
Expand Down
38 changes: 38 additions & 0 deletions backend/tests/test_crud_entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,22 @@ def test_no_raise_on_non_unique_if_existing_is_deleted(self, dbsession):
e = dbsession.execute(select(Entity).where(Entity.slug == 'Jack')).scalar()
assert e.get('nickname', dbsession).value == 'jane'

def test_restore(self, dbsession):
"""
Test that updating a deleted entity restores it implicitly
"""
e = self.get_default_entity(dbsession)
delete_entity(dbsession, id_or_slug=e.id, schema_id=e.schema_id)

dbsession.refresh(e)
assert e.deleted is True

data = {'slug': e.slug}
update_entity(dbsession, id_or_slug=e.id, schema_id=e.schema_id, data=data)

dbsession.refresh(e)
assert e.deleted is False


class TestEntityDelete(DefaultMixin):
def asserts_after_entity_delete(self, db: Session):
Expand Down Expand Up @@ -667,3 +683,25 @@ def test_raise_on_already_deleted(self, dbsession):
dbsession.flush()
with pytest.raises(MissingEntityException):
delete_entity(dbsession, id_or_slug=entity.id, schema_id=entity.schema_id)


class TestRestoreEntity(DefaultMixin):
def asserts_after_entity_restore(self, db: Session):
entities = db.execute(select(Entity)).scalars().all()
assert len(entities) == 2
e = self.get_default_entity(db)
assert e.deleted is False

def test_restore_by_id(self, dbsession):
e = self.get_default_entity(dbsession)
e.deleted = True
dbsession.flush()
restore_entity(dbsession, id_or_slug=e.id, schema_id=e.schema_id)
self.asserts_after_entity_restore(db=dbsession)

def test_restore_by_slug(self, dbsession):
e = self.get_default_entity(dbsession)
e.deleted = True
dbsession.flush()
restore_entity(dbsession, id_or_slug=e.slug, schema_id=e.schema_id)
self.asserts_after_entity_restore(db=dbsession)
36 changes: 33 additions & 3 deletions backend/tests/test_dynamic_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,13 +422,28 @@ def test_raise_on_entity_doesnt_exist(self, dbsession, authorized_client):
assert response.status_code == 404
assert "doesn't exist or was deleted" in response.json()['detail']

def test_raise_on_schema_is_deleted(self, dbsession, authorized_client):
def test_restore(self, dbsession, authorized_client):
entity = self.get_default_entity(dbsession)
entity.deleted = True
dbsession.commit()

data = {'slug': entity.slug, 'name': 'Foo Bær'}
response = authorized_client.put(f'entity/person/{entity.id}', json=data)
crazyscientist marked this conversation as resolved.
Show resolved Hide resolved
dbsession.refresh(entity)
assert entity.deleted is False
assert response.status_code == 200
assert response.json() == {'id': entity.id, 'slug': entity.slug, 'name': 'Foo Bær',
'deleted': False}

def test_restore__no_change(self, dbsession, authorized_client):
entity = self.get_default_entity(dbsession)
entity.deleted = True
dbsession.commit()
response = authorized_client.put(f'/entity/person/{entity.id}', json={})
assert response.status_code == 404
assert "doesn't exist or was deleted" in response.json()['detail']
dbsession.refresh(entity)
assert entity.deleted is True
assert response.status_code == 208
assert "contains no changes" in response.json()['detail']

def test_raise_on_entity_already_exists(self, dbsession, authorized_client):
data = {'slug': 'Jane'}
Expand Down Expand Up @@ -528,3 +543,18 @@ def test_raise_on_already_deleted(self, dbsession, authorized_client):
dbsession.commit()
response = authorized_client.delete(f'/entity/person/{entity.id}')
assert response.status_code == 404

def test_restore(self, dbsession, authorized_client):
entity = self.get_default_entity(dbsession)
entity.deleted = True
dbsession.commit()
dbsession.refresh(entity)
assert entity.deleted is True
response = authorized_client.delete(f'/entity/person/{entity.slug}?restore=1')
assert response.status_code == 200
assert response.json() == {'id': entity.id, 'slug': 'Jack', 'name': 'Jack', 'deleted': False}
crazyscientist marked this conversation as resolved.
Show resolved Hide resolved

def test_restore_on_nondeleted(self, dbsession, authorized_client):
entity = self.get_default_entity(dbsession)
response = authorized_client.delete(f'/entity/person/{entity.slug}?restore=1')
assert response.status_code == 404
70 changes: 69 additions & 1 deletion backend/traceability/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from ..enum import ModelVariant
from ..exceptions import MissingChangeException, MissingEntityCreateRequestException, \
AttributeNotDefinedException, MissingEntityUpdateRequestException, NoOpChangeException, \
MissingEntityDeleteRequestException, MissingChangeRequestException
MissingEntityDeleteRequestException, MissingChangeRequestException, \
MissingEntityRestoreRequestException
from ..models import Entity, AttributeDefinition, Schema, Attribute
from ..schemas.entity import EntityModelFactory
from ..schemas.traceability import EntityChangeDetailSchema
Expand Down Expand Up @@ -538,3 +539,70 @@ def apply_entity_delete_request(db: Session, change_request: ChangeRequest, revi
change_request.comment = comment
db.commit()
return True, entity


def create_entity_restore_request(db: Session, id_or_slug: Union[int, str], schema_id: int,
created_by: User, commit: bool = True) -> ChangeRequest:
crud.restore_entity(db=db, id_or_slug=id_or_slug, schema_id=schema_id, commit=False)
db.rollback()
schema = crud.get_schema(db=db, id_or_slug=schema_id)
entity = crud.get_entity_model(db=db, id_or_slug=id_or_slug, schema=schema)

change_request = ChangeRequest(
created_by=created_by,
created_at=datetime.now(timezone.utc),
object_type=EditableObjectType.ENTITY,
object_id=entity.id,
change_type=ChangeType.RESTORE
)
db.add(change_request)

val = ChangeValueBool(old_value=entity.deleted, new_value=False)
db.add(val)
db.flush()
change = Change(
change_request=change_request,
data_type=ChangeAttrType.BOOL,
change_type=ChangeType.RESTORE,
content_type=ContentType.ENTITY,
object_id=entity.id,
field_name='deleted',
value_id=val.id
)
db.add(change)
if commit:
db.commit()
else:
db.flush()
return change_request


def apply_entity_restore_request(db: Session, change_request: ChangeRequest, reviewed_by: User,
comment: Optional[str]) -> Tuple[bool, Entity]:
change = db.execute(
select(Change)
.where(Change.change_request_id == change_request.id)
.where(Change.data_type == ChangeAttrType.BOOL)
.where(Change.change_type == ChangeType.RESTORE)
.where(Change.content_type == ContentType.ENTITY)
.where(Change.field_name == 'deleted')
.where(Change.object_id != None)
).scalar()

if change is None:
raise MissingEntityRestoreRequestException(obj_id=change_request.id)
entity = crud.get_entity_by_id(db=db, entity_id=change.object_id)
v = db.execute(select(ChangeValueBool).where(ChangeValueBool.id == change.value_id)).scalar()
v.old_value = entity.deleted
entity = crud.restore_entity(
db=db,
id_or_slug=entity.id,
schema_id=entity.schema_id,
commit=False
)
change_request.status = ChangeStatus.APPROVED
change_request.reviewed_by = reviewed_by
change_request.reviewed_at = datetime.now(timezone.utc)
change_request.comment = comment
db.commit()
return True, entity
1 change: 1 addition & 0 deletions backend/traceability/enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ class ChangeType(enum.Enum):
CREATE = 'CREATE'
UPDATE = 'UPDATE'
DELETE = 'DELETE'
RESTORE = 'RESTORE'
21 changes: 20 additions & 1 deletion frontend/src/components/EntityList.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<template>
<SearchPanel
ref="searchpanel"
@search="setFiltersAndSearch"
:key="schema?.slug"
:schema="schema"
Expand All @@ -22,12 +23,21 @@
{{numSelected}} {{ entityPluralized }} selected
</small>
<ConfirmButton :callback="onDeletion"
btn-class="btn-outline-danger">
btn-class="btn-outline-danger"
v-if="$refs?.searchpanel?.listMode !== 'deleted'">
<template v-slot:label>
<i class="eos-icons me-1">delete</i>
<span>Delete</span>
</template>
</ConfirmButton>
<ConfirmButton :callback="onRestoration"
btn-class="btn-outline-primary"
v-if="$refs?.searchpanel?.listMode !== 'active'">
der-gabe marked this conversation as resolved.
Show resolved Hide resolved
<template v-slot:label>
<i class="eos-icons me-1">restore_from_trash</i>
<span>Restore</span>
</template>
</ConfirmButton>
<RouterLink class="btn btn-outline-primary"
:to="{name: 'bulk-edit', query: {entity: selected}}">
<i class="eos-icons me-1">edit</i>
Expand Down Expand Up @@ -150,6 +160,15 @@ export default {
});
});
Promise.all(promises).then(() => this.getEntities({resetPage: true}));
},
onRestoration() {
const promises = this.selected.map(eId => {
this.$api.restoreEntity({
schemaSlug: this.schema.slug,
entityIdOrSlug: eId
});
});
Promise.all(promises).then(() => this.getEntities({resetPage: true}));
}
},
data() {
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/components/EntityListTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
<tr v-for="e in entities" :key="e.id" :class="{'table-light': e.deleted}">
<td v-if="showSelectors">
<input :type="inputType" class="form-check-input" name="EntitySelection" :value="e.id"
@change="$emit('select')" :checked="selected.includes(e.id)"
:disabled="e.deleted"/>
@change="$emit('select')" :checked="selected.includes(e.id)" />
</td>
<td v-for="field in displayFieldsWithDescriptions" :key="field.name">
<template v-if="field.name === 'name'">
Expand Down
33 changes: 31 additions & 2 deletions frontend/src/components/inputs/EntityForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,18 @@
</div>
</template>
</template>
<div class="d-grid gap-2 mt-1">
<button type="button" class="btn btn-primary p-2" @click="saveChanges">
<div class="d-flex gap-2 mt-1">
<button v-if="entity?.id && !entity?.deleted" type="button" class="btn btn-danger p-2"
@click="deleteEntity">
<i class="eos-icons me-1">delete</i>
Delete
</button>
<button v-if="entity?.id && entity?.deleted" type="button" class="btn btn-primary p-2"
@click="restoreEntity">
<i class="eos-icons me-1">restore_from_trash</i>
Restore
</button>
<button type="button" class="btn btn-primary p-2 flex-grow-1" @click="saveChanges">
<i class="eos-icons me-1">save</i>
Save {{ batchMode? 'all changes' : 'changes'}}
</button>
Expand Down Expand Up @@ -231,7 +241,26 @@ export default {
if (response) {
this.updatePendingRequests();
}
this.$emit("update");
der-gabe marked this conversation as resolved.
Show resolved Hide resolved
},
async deleteEntity() {
if (this.entity?.id) {
await this.$api.deleteEntity({
schemaSlug: this.schema.slug,
entityIdOrSlug: this.entity.id
});
this.$emit("update");
}
},
async restoreEntity() {
if (this.entity?.id) {
await this.$api.restoreEntity({
schemaSlug: this.schema.slug,
entityIdOrSlug: this.entity.id
});
this.$emit("update");
}
}
},
}
</script>
Expand Down
Loading