From edc0e6f8599fc0e92789dfc1394adcf70f2f7972 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Wed, 26 Feb 2025 10:09:41 -0800 Subject: [PATCH] Fix: cleanup saved view references on custom field deletion, auto-refresh views, show error on saved view save (#9225) --- src-ui/messages.xlf | 70 +++++++++++-------- .../document-list.component.spec.ts | 26 ++++++- .../document-list/document-list.component.ts | 20 ++++-- .../custom-fields/custom-fields.component.ts | 5 +- src/documents/serialisers.py | 5 +- src/documents/signals/handlers.py | 28 ++++++++ src/documents/tests/test_api_documents.py | 42 +++++++++-- 7 files changed, 154 insertions(+), 42 deletions(-) diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf index ae9abe847..12b0a09a6 100644 --- a/src-ui/messages.xlf +++ b/src-ui/messages.xlf @@ -1736,7 +1736,7 @@ src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 83 + 87 src/app/components/document-list/document-list.component.html @@ -2230,7 +2230,7 @@ src/app/components/manage/custom-fields/custom-fields.component.ts - 106 + 108 src/app/components/manage/mail/mail.component.ts @@ -2565,7 +2565,7 @@ src/app/components/manage/custom-fields/custom-fields.component.ts - 108 + 110 src/app/components/manage/mail/mail.component.ts @@ -3322,7 +3322,7 @@ src/app/components/manage/custom-fields/custom-fields.component.ts - 87 + 89 @@ -3333,7 +3333,7 @@ src/app/components/manage/custom-fields/custom-fields.component.ts - 96 + 98 @@ -3543,7 +3543,7 @@ src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 79 + 83 src/app/components/document-list/document-list.component.html @@ -4384,7 +4384,7 @@ src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 125 + 129 src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.html @@ -4984,11 +4984,18 @@ 72 + + Web UI + + src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts + 76 + + Modified src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 87 + 91 src/app/data/document.ts @@ -4999,70 +5006,70 @@ Custom Field src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 91 + 95 Consumption Started src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 98 + 102 Document Added src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 102 + 106 Document Updated src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 106 + 110 Scheduled src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 110 + 114 Assignment src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 117 + 121 Removal src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 121 + 125 Webhook src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 129 + 133 Create new workflow src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 225 + 229 Edit workflow src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts - 229 + 233 @@ -7870,14 +7877,21 @@ View "" saved successfully. src/app/components/document-list/document-list.component.ts - 383 + 384 + + + + Failed to save view "". + + src/app/components/document-list/document-list.component.ts + 390 View "" created successfully. src/app/components/document-list/document-list.component.ts - 426 + 434 @@ -8227,28 +8241,28 @@ Confirm delete field src/app/components/manage/custom-fields/custom-fields.component.ts - 104 + 106 This operation will permanently delete this field. src/app/components/manage/custom-fields/custom-fields.component.ts - 105 + 107 Deleted field "" src/app/components/manage/custom-fields/custom-fields.component.ts - 114 + 116 Error deleting field "". src/app/components/manage/custom-fields/custom-fields.component.ts - 122 + 125 @@ -9726,28 +9740,28 @@ Connecting... src/app/services/upload-documents.service.ts - 42 + 43 Uploading... src/app/services/upload-documents.service.ts - 54 + 55 Upload complete, waiting... src/app/services/upload-documents.service.ts - 57 + 58 HTTP error: src/app/services/upload-documents.service.ts - 70 + 71 diff --git a/src-ui/src/app/components/document-list/document-list.component.spec.ts b/src-ui/src/app/components/document-list/document-list.component.spec.ts index 13a938f59..aae043fdb 100644 --- a/src-ui/src/app/components/document-list/document-list.component.spec.ts +++ b/src-ui/src/app/components/document-list/document-list.component.spec.ts @@ -376,7 +376,7 @@ describe('DocumentListComponent', () => { expect(documentListService.selected.size).toEqual(3) }) - it('should support saving an edited view', () => { + it('should support saving a view', () => { const view: SavedView = { id: 10, name: 'Saved View 10', @@ -414,6 +414,30 @@ describe('DocumentListComponent', () => { ) }) + it('should handle error on view saving', () => { + component.list.activateSavedView({ + id: 10, + name: 'Saved View 10', + sort_field: 'added', + sort_reverse: true, + filter_rules: [ + { + rule_type: FILTER_HAS_TAGS_ANY, + value: '20', + }, + ], + }) + const toastErrorSpy = jest.spyOn(toastService, 'showError') + jest + .spyOn(savedViewService, 'patch') + .mockReturnValueOnce(throwError(() => new Error('Error saving view'))) + component.saveViewConfig() + expect(toastErrorSpy).toHaveBeenCalledWith( + 'Failed to save view "Saved View 10".', + expect.any(Error) + ) + }) + it('should support edited view saving as', () => { const view: SavedView = { id: 10, diff --git a/src-ui/src/app/components/document-list/document-list.component.ts b/src-ui/src/app/components/document-list/document-list.component.ts index e1f71edbc..f6b7c181b 100644 --- a/src-ui/src/app/components/document-list/document-list.component.ts +++ b/src-ui/src/app/components/document-list/document-list.component.ts @@ -377,12 +377,20 @@ export class DocumentListComponent this.savedViewService .patch(savedView) .pipe(first()) - .subscribe((view) => { - this.unmodifiedSavedView = view - this.toastService.showInfo( - $localize`View "${this.list.activeSavedViewTitle}" saved successfully.` - ) - this.unmodifiedFilterRules = this.list.filterRules + .subscribe({ + next: (view) => { + this.unmodifiedSavedView = view + this.toastService.showInfo( + $localize`View "${this.list.activeSavedViewTitle}" saved successfully.` + ) + this.unmodifiedFilterRules = this.list.filterRules + }, + error: (err) => { + this.toastService.showError( + $localize`Failed to save view "${this.list.activeSavedViewTitle}".`, + err + ) + }, }) } } diff --git a/src-ui/src/app/components/manage/custom-fields/custom-fields.component.ts b/src-ui/src/app/components/manage/custom-fields/custom-fields.component.ts index a431453d4..b4fd9738d 100644 --- a/src-ui/src/app/components/manage/custom-fields/custom-fields.component.ts +++ b/src-ui/src/app/components/manage/custom-fields/custom-fields.component.ts @@ -17,6 +17,7 @@ import { DocumentListViewService } from 'src/app/services/document-list-view.ser import { PermissionsService } from 'src/app/services/permissions.service' import { CustomFieldsService } from 'src/app/services/rest/custom-fields.service' import { DocumentService } from 'src/app/services/rest/document.service' +import { SavedViewService } from 'src/app/services/rest/saved-view.service' import { SettingsService } from 'src/app/services/settings.service' import { ToastService } from 'src/app/services/toast.service' import { ConfirmDialogComponent } from '../../common/confirm-dialog/confirm-dialog.component' @@ -50,7 +51,8 @@ export class CustomFieldsComponent private toastService: ToastService, private documentListViewService: DocumentListViewService, private settingsService: SettingsService, - private documentService: DocumentService + private documentService: DocumentService, + private savedViewService: SavedViewService ) { super() } @@ -115,6 +117,7 @@ export class CustomFieldsComponent this.customFieldsService.clearCache() this.settingsService.initializeDisplayFields() this.documentService.reload() + this.savedViewService.reload() this.reload() }, error: (e) => { diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 5f3b310c2..a486fe241 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1136,8 +1136,9 @@ class SavedViewSerializer(OwnedObjectSerializer): ): # i.e. check for 'custom_field_' prefix field_id = int(re.search(r"\d+", field)[0]) if not CustomField.objects.filter(id=field_id).exists(): - # In case the field was deleted, just remove from the list - attrs["display_fields"].remove(field) + raise serializers.ValidationError( + f"Invalid field: {field}", + ) elif field not in SavedView.DisplayFields.values: raise serializers.ValidationError( f"Invalid field: {field}", diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 4345e04d5..b3f029da7 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -36,6 +36,7 @@ from documents.models import Document from documents.models import DocumentType from documents.models import MatchingModel from documents.models import PaperlessTask +from documents.models import SavedView from documents.models import Tag from documents.models import Workflow from documents.models import WorkflowAction @@ -549,6 +550,33 @@ def check_paths_and_prune_custom_fields(sender, instance: CustomField, **kwargs) update_filename_and_move_files(sender, cf_instance) +@receiver(models.signals.post_delete, sender=CustomField) +def cleanup_custom_field_deletion(sender, instance: CustomField, **kwargs): + """ + When a custom field is deleted, ensure no saved views reference it. + """ + field_identifier = SavedView.DisplayFields.CUSTOM_FIELD % instance.pk + # remove field from display_fields of all saved views + for view in SavedView.objects.filter(display_fields__isnull=False).distinct(): + if field_identifier in view.display_fields: + logger.debug( + f"Removing custom field {instance} from view {view}", + ) + view.display_fields.remove(field_identifier) + view.save() + + # remove from sort_field of all saved views + views_with_sort_updated = SavedView.objects.filter( + sort_field=field_identifier, + ).update( + sort_field=SavedView.DisplayFields.CREATED, + ) + if views_with_sort_updated > 0: + logger.debug( + f"Removing custom field {instance} from sort field of {views_with_sort_updated} views", + ) + + def add_to_index(sender, document, **kwargs): from documents import index diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index 40c30f5bb..cd923b281 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -1911,7 +1911,7 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase): ], ) - # Custom field not found, removed from list + # Custom field not found response = self.client.patch( f"/api/saved_views/{v1.id}/", { @@ -1923,9 +1923,43 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase): }, format="json", ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - v1.refresh_from_db() - self.assertNotIn(SavedView.DisplayFields.CUSTOM_FIELD % 99, v1.display_fields) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_saved_view_cleanup_after_custom_field_deletion(self): + """ + GIVEN: + - Saved view with custom field in display fields and as sort field + WHEN: + - Custom field is deleted + THEN: + - Custom field is removed from display fields and sort field + """ + custom_field = CustomField.objects.create( + name="stringfield", + data_type=CustomField.FieldDataType.STRING, + ) + + view = SavedView.objects.create( + owner=self.user, + name="test", + sort_field=SavedView.DisplayFields.CUSTOM_FIELD % custom_field.id, + show_on_dashboard=True, + show_in_sidebar=True, + display_fields=[ + SavedView.DisplayFields.TITLE, + SavedView.DisplayFields.CREATED, + SavedView.DisplayFields.CUSTOM_FIELD % custom_field.id, + ], + ) + + custom_field.delete() + + view.refresh_from_db() + self.assertEqual(view.sort_field, SavedView.DisplayFields.CREATED) + self.assertEqual( + view.display_fields, + [str(SavedView.DisplayFields.TITLE), str(SavedView.DisplayFields.CREATED)], + ) def test_get_logs(self): log_data = "test\ntest2\n"