Fix: cleanup saved view references on custom field deletion, auto-refresh views, show error on saved view save (#9225)

This commit is contained in:
shamoon 2025-02-26 10:09:41 -08:00 committed by GitHub
parent 61cb5103ed
commit edc0e6f859
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 154 additions and 42 deletions

View File

@ -1736,7 +1736,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">83</context>
<context context-type="linenumber">87</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/document-list.component.html</context>
@ -2230,7 +2230,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">106</context>
<context context-type="linenumber">108</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/mail/mail.component.ts</context>
@ -2565,7 +2565,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">108</context>
<context context-type="linenumber">110</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/mail/mail.component.ts</context>
@ -3322,7 +3322,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">87</context>
<context context-type="linenumber">89</context>
</context-group>
</trans-unit>
<trans-unit id="1841172489943868696" datatype="html">
@ -3333,7 +3333,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">96</context>
<context context-type="linenumber">98</context>
</context-group>
</trans-unit>
<trans-unit id="6048892649018070225" datatype="html">
@ -3543,7 +3543,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">79</context>
<context context-type="linenumber">83</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/document-list.component.html</context>
@ -4384,7 +4384,7 @@
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">125</context>
<context context-type="linenumber">129</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.html</context>
@ -4984,11 +4984,18 @@
<context context-type="linenumber">72</context>
</context-group>
</trans-unit>
<trans-unit id="235571817610183244" datatype="html">
<source>Web UI</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">76</context>
</context-group>
</trans-unit>
<trans-unit id="3553216189604488439" datatype="html">
<source>Modified</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">87</context>
<context context-type="linenumber">91</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/data/document.ts</context>
@ -4999,70 +5006,70 @@
<source>Custom Field</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">91</context>
<context context-type="linenumber">95</context>
</context-group>
</trans-unit>
<trans-unit id="8696908693776094667" datatype="html">
<source>Consumption Started</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">98</context>
<context context-type="linenumber">102</context>
</context-group>
</trans-unit>
<trans-unit id="7858311467093621703" datatype="html">
<source>Document Added</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">102</context>
<context context-type="linenumber">106</context>
</context-group>
</trans-unit>
<trans-unit id="7955486237346046731" datatype="html">
<source>Document Updated</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">106</context>
<context context-type="linenumber">110</context>
</context-group>
</trans-unit>
<trans-unit id="9172233176401579786" datatype="html">
<source>Scheduled</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">110</context>
<context context-type="linenumber">114</context>
</context-group>
</trans-unit>
<trans-unit id="5502398334173581061" datatype="html">
<source>Assignment</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">117</context>
<context context-type="linenumber">121</context>
</context-group>
</trans-unit>
<trans-unit id="6234812824772766804" datatype="html">
<source>Removal</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">121</context>
<context context-type="linenumber">125</context>
</context-group>
</trans-unit>
<trans-unit id="4206419737792796794" datatype="html">
<source>Webhook</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">129</context>
<context context-type="linenumber">133</context>
</context-group>
</trans-unit>
<trans-unit id="3138206142174978019" datatype="html">
<source>Create new workflow</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">225</context>
<context context-type="linenumber">229</context>
</context-group>
</trans-unit>
<trans-unit id="5996779210524133604" datatype="html">
<source>Edit workflow</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/edit-dialog/workflow-edit-dialog/workflow-edit-dialog.component.ts</context>
<context context-type="linenumber">229</context>
<context context-type="linenumber">233</context>
</context-group>
</trans-unit>
<trans-unit id="7376342558017986274" datatype="html">
@ -7870,14 +7877,21 @@
<source>View &quot;<x id="PH" equiv-text="this.list.activeSavedViewTitle"/>&quot; saved successfully.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/document-list.component.ts</context>
<context context-type="linenumber">383</context>
<context context-type="linenumber">384</context>
</context-group>
</trans-unit>
<trans-unit id="4646273665293421938" datatype="html">
<source>Failed to save view &quot;<x id="PH" equiv-text="this.list.activeSavedViewTitle"/>&quot;.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/document-list.component.ts</context>
<context context-type="linenumber">390</context>
</context-group>
</trans-unit>
<trans-unit id="6837554170707123455" datatype="html">
<source>View &quot;<x id="PH" equiv-text="savedView.name"/>&quot; created successfully.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/document-list.component.ts</context>
<context context-type="linenumber">426</context>
<context context-type="linenumber">434</context>
</context-group>
</trans-unit>
<trans-unit id="739880801667335279" datatype="html">
@ -8227,28 +8241,28 @@
<source>Confirm delete field</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">104</context>
<context context-type="linenumber">106</context>
</context-group>
</trans-unit>
<trans-unit id="2939457975223185057" datatype="html">
<source>This operation will permanently delete this field.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">105</context>
<context context-type="linenumber">107</context>
</context-group>
</trans-unit>
<trans-unit id="4679555638382452936" datatype="html">
<source>Deleted field &quot;<x id="PH" equiv-text="field.name"/>&quot;</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">114</context>
<context context-type="linenumber">116</context>
</context-group>
</trans-unit>
<trans-unit id="4704551499967874824" datatype="html">
<source>Error deleting field &quot;<x id="PH" equiv-text="field.name"/>&quot;.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/manage/custom-fields/custom-fields.component.ts</context>
<context context-type="linenumber">122</context>
<context context-type="linenumber">125</context>
</context-group>
</trans-unit>
<trans-unit id="8084492669582894778" datatype="html">
@ -9726,28 +9740,28 @@
<source>Connecting...</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/services/upload-documents.service.ts</context>
<context context-type="linenumber">42</context>
<context context-type="linenumber">43</context>
</context-group>
</trans-unit>
<trans-unit id="1245343823699368872" datatype="html">
<source>Uploading...</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/services/upload-documents.service.ts</context>
<context context-type="linenumber">54</context>
<context context-type="linenumber">55</context>
</context-group>
</trans-unit>
<trans-unit id="7446520539098045935" datatype="html">
<source>Upload complete, waiting...</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/services/upload-documents.service.ts</context>
<context context-type="linenumber">57</context>
<context context-type="linenumber">58</context>
</context-group>
</trans-unit>
<trans-unit id="1405142710727603568" datatype="html">
<source>HTTP error: <x id="PH" equiv-text="error.status"/> <x id="PH_1" equiv-text="error.statusText"/></source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/services/upload-documents.service.ts</context>
<context context-type="linenumber">70</context>
<context context-type="linenumber">71</context>
</context-group>
</trans-unit>
<trans-unit id="2119857572761283468" datatype="html">

View File

@ -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,

View File

@ -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
)
},
})
}
}

View File

@ -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) => {

View File

@ -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}",

View File

@ -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

View File

@ -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"