Single table for file attachments #7420

SchrodingersGat · 2024-06-08T12:31:53Z

Refactoring how "attachments" are stored, moving to a single table approach.

Tasks

Related Issues

netlify · 2024-06-08T12:32:10Z

✅ Deploy Preview for inventree-web-pui-preview canceled.

Name	Link
🔨 Latest commit	`c94d901`
🔍 Latest deploy log	https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6672569f8b59f00008159947

codecov · 2024-06-08T12:51:06Z

Codecov Report

Attention: Patch coverage is 86.63283% with 79 lines in your changes missing coverage. Please review.

Project coverage is 84.01%. Comparing base (79ea689) to head (c94d901).
Report is 344 commits behind head on master.

Files with missing lines	Patch %	Lines
src/backend/InvenTree/common/models.py	76.92%	18 Missing ⚠️
...nTree/common/migrations/0026_auto_20240608_1238.py	62.79%	16 Missing ⚠️
src/frontend/src/states/UserState.tsx	33.33%	11 Missing and 3 partials ⚠️
...rc/frontend/src/tables/general/AttachmentTable.tsx	60.00%	7 Missing and 1 partial ⚠️
src/backend/InvenTree/common/api.py	84.61%	6 Missing ⚠️
src/frontend/src/defaults/formatters.tsx	14.28%	6 Missing ⚠️
src/backend/InvenTree/common/admin.py	82.35%	3 Missing ⚠️
src/backend/InvenTree/common/settings.py	50.00%	2 Missing ⚠️
src/backend/InvenTree/common/validators.py	85.71%	2 Missing ⚠️
src/backend/InvenTree/common/serializers.py	96.42%	1 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7420      +/-   ##
==========================================
+ Coverage   83.84%   84.01%   +0.16%     
==========================================
  Files        1058     1066       +8     
  Lines       46442    46618     +176     
  Branches     1386     1389       +3     
==========================================
+ Hits        38941    39166     +225     
+ Misses       7141     7092      -49     
  Partials      360      360

Flag	Coverage Δ
backend	`85.18% <79.07%> (+0.03%)`	⬆️
migrations	`42.50% <55.18%> (-0.06%)`	⬇️
pui	`65.66% <45.09%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/backend/InvenTree/common/models.py

- Add new mixin class to designate which models can have attachments

- Ensure other apps are at the correct migration state beforehand

- ContentType does not allow easy API serialization

matmair · 2024-06-17T06:09:40Z

Looks like the migrations might be tainted? Tough I can not see anything modifying the DB state outside of migrations.

The problem I see is that the error is raised in core Django itself so guarding against it might be difficult/impossible.

- Ensure all apps are "up to date" before removing legacy tables

…enTree into attachments-refactor

- Seems to play havoc with bootup sequence

- Use get_global_setting

SchrodingersGat · 2024-06-18T04:36:20Z

@matmair all passing now, ready for merge if you're happy :)

matmair

LGTM; Great effort!

matmair · 2024-06-18T21:43:53Z

src/backend/InvenTree/InvenTree/fields.py

@@ -33,7 +33,7 @@ def __init__(self, **kwargs):

    def run_validation(self, data=empty):
        """Override default validation behaviour for this field type."""
-        strict_urls = get_global_setting('INVENTREE_STRICT_URLS', True, cache=False)
+        strict_urls = get_global_setting('INVENTREE_STRICT_URLS', cache=False)


Q: Why is this changed?

The default value filters through from the definition of the setting itself, and does not need to be passed here.

matmair · 2024-06-18T21:45:59Z

src/backend/InvenTree/InvenTree/models.py

-        forbidden = [
-            "'",
-            '"',
-            '#',
-            '@',
-            '!',
-            '&',
-            '^',
-            '<',
-            '>',
-            ':',
-            ';',
-            '/',
-            '\\',
-            '|',
-            '?',
-            '*',
-            '%',
-            '~',
-            '`',
-        ]


We did not add checks when we fixed the original issues as there was some sever time pressure. I will try to add unit tests for unsafe uploads to my todo

matmair · 2024-06-18T21:47:20Z

src/backend/InvenTree/InvenTree/version.py

+    if get_global_setting('INVENTREE_INSTANCE_TITLE'):
+        return get_global_setting('INVENTREE_INSTANCE')


Q: Why are these defualts changed?

As above, duplicating the default value here is not required.

matmair · 2024-06-18T21:52:54Z

src/backend/InvenTree/report/tests.py

@@ -502,7 +502,7 @@ def run_print_test(self, qs, model_type, label: bool = True):
            },
            expected_code=201,
            max_query_time=15,
-            max_query_count=1000,  # TODO: Should look into this
+            max_query_count=500 * len(qs),


C: 500 queries per entry seems a bit much - do we need to optimise this endpoint?

The label printing is pretty inefficient, but there's a lot of work required to profile exactly why

matmair · 2024-06-18T21:54:47Z

src/backend/InvenTree/plugin/registry.py

@@ -786,7 +786,7 @@ def calculate_plugin_hash(self):

        for k in self.plugin_settings_keys():
            try:
-                val = get_global_setting(k, False, create=False)
+                val = get_global_setting(k)


Q: Why were the defaults removed?

matmair · 2024-06-18T21:57:00Z

One additonal comment: I do not think #5429 is really covered by this but would be closed - wdyt?

SchrodingersGat · 2024-06-18T23:21:45Z

With regard to #5429 - we can add the "tags" mixin to the new attachment model, and then the tags can be used for custom data storage.

Potentially could add metadata also.

matmair · 2024-06-19T06:29:29Z

Thank you for the addition!

* Add basic model for handling generic attachments * Refactor migration * Data migration to convert old files across * Admin updates * Increase comment field max_length * Adjust field name * Remove legacy serializer classes / endpoints * Expose new model to API * Admin site list filters * Remove legacy attachment models - Add new mixin class to designate which models can have attachments * Update data migration - Ensure other apps are at the correct migration state beforehand * Add migrations to remove legacy attachment tables * Fix for "rename_attachment" callback * Refactor model_type field - ContentType does not allow easy API serialization * Set allowed options for admin * Update model verbose names * Fix logic for file upload * Add choices for serializer * Add API filtering * Fix for API filter * Fix for attachment tables in PUI - Still not solved permission issues * Bump API version * Record user when uploading attachment via API * Refactor <AttachmentTable /> for PUI * Display 'file_size' in PUI attachment table * Fix company migrations * Include permission informtion in roles API endpoint * Read user permissions in PUI * Simplify permission checks for <AttachmentTable /> * Automatically clean up old content types * Cleanup PUI * Fix typo in data migration * Add reverse data migration * Update unit tests * Use InMemoryStorage for media files in test mode * Data migration unit test * Fix "model_type" field - It is a required field after all * Add permission check for serializer * Fix permission check for CUI * Fix PUI import * Test python lib against specific branch - Will be reverted once code is merged * Revert STORAGES setting - Might be worth looking into again * Fix part unit test * Fix unit test for sales order * Use 'get_global_setting' * Use 'get_global_setting' * Update setting getter * Unit tests * Tweaks * Revert change to settings.py * More updates for get_global_setting * Relax API query count requirement * remove illegal chars and add unit tests * Fix unit tests * Fix frontend unit tests * settings management updates * Prevent db write under more conditions * Simplify settings code * Pop values before creating filters * Prevent settings write under certain conditions * Add debug msg * Clear db on record import * Refactor permissions checks - Allows extension / customization of permission checks at a later date * Unit test updates * Prevent delete of attachment without correct permissions * Adjust odcker.yaml * Cleanup data migrations * Tweak migration tests for build app * Update data migration - Handle case with missing data * Prevent debug shell in TESTING mode * Update migration dependencies - Ensure all apps are "up to date" before removing legacy tables * add file size test * Update migration tests * Revert some settings caching changes * Fix incorrect logic in migration * Update unit tests * Prevent create on CURRENCY_CODES - Seems to play havoc with bootup sequence * Fix unit test * Some refactoring - Use get_global_setting * Fix typo * Revert change * Add "tags" and "metadata" * Include "tags" field in API serializer * add "metadata" endpoint for attachments

Add basic model for handling generic attachments

0bb25b2

SchrodingersGat added enhancement This is an suggested enhancement or new feature api Relates to the API breaking Indicates a major update or change which breaks compatibility refactor migration Data or schema migrations labels Jun 8, 2024

SchrodingersGat added this to the 0.16.0 milestone Jun 8, 2024

Refactor migration

3fb7d18

SchrodingersGat added 3 commits June 8, 2024 13:04

Data migration to convert old files across

ae5c38c

Admin updates

79d5bc1

Merge remote-tracking branch 'origin/master' into attachments-refactor

9e34b67

matmair approved these changes Jun 8, 2024

View reviewed changes

src/backend/InvenTree/common/models.py Outdated Show resolved Hide resolved

src/backend/InvenTree/common/models.py Outdated Show resolved Hide resolved

src/backend/InvenTree/common/models.py Show resolved Hide resolved

SchrodingersGat added 16 commits June 9, 2024 07:24

Increase comment field max_length

5250508

Adjust field name

8031a49

Remove legacy serializer classes / endpoints

e50ab6e

Expose new model to API

a4f4280

Admin site list filters

4ca4638

Remove legacy attachment models

66dcee4

- Add new mixin class to designate which models can have attachments

Update data migration

3cc7dd0

- Ensure other apps are at the correct migration state beforehand

Add migrations to remove legacy attachment tables

b589ef0

Fix for "rename_attachment" callback

353f1dc

Refactor model_type field

04e978e

- ContentType does not allow easy API serialization

Set allowed options for admin

1f6d4b0

Update model verbose names

def177d

Fix logic for file upload

ee59ca3

Add choices for serializer

6c8f36b

Add API filtering

f390780

Fix for API filter

8f3414f

SchrodingersGat added 14 commits June 17, 2024 20:46

Merge branch 'master' into attachments-refactor

f93d898

Update migration dependencies

da258d9

- Ensure all apps are "up to date" before removing legacy tables

add file size test

e164cd3

Update migration tests

80f06ad

Merge branch 'master' into attachments-refactor

e09394d

Revert some settings caching changes

0f867dd

Fix incorrect logic in migration

e1c424c

Update unit tests

b4767e9

Merge branch 'attachments-refactor' of github.com:SchrodingersGat/Inv…

3d31689

…enTree into attachments-refactor

Prevent create on CURRENCY_CODES

b713d3d

- Seems to play havoc with bootup sequence

Fix unit test

28d5ade

Some refactoring

a17caa6

- Use get_global_setting

Fix typo

18e86ca

Revert change

7dc5403

matmair approved these changes Jun 18, 2024

View reviewed changes

SchrodingersGat added 3 commits June 19, 2024 03:43

Add "tags" and "metadata"

90bad53

Include "tags" field in API serializer

fda2381

add "metadata" endpoint for attachments

c94d901

SchrodingersGat merged commit 432e0c6 into inventree:master Jun 19, 2024
27 checks passed

SchrodingersGat deleted the attachments-refactor branch June 19, 2024 04:38

This was referenced Jun 24, 2024

Attachment refactor afkiwers/inventree_kicad#98

Merged

Handle modern attachment API inventree/inventree-wireviz#41

Merged

wolflu05 mentioned this pull request Jul 15, 2024

Fix part attachment wolflu05/inventree-bulk-plugin#78

Merged

matmair mentioned this pull request Aug 6, 2024

Missing API endpoint #7818

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single table for file attachments #7420

Single table for file attachments #7420

SchrodingersGat commented Jun 8, 2024 •

edited

Loading

netlify bot commented Jun 8, 2024 •

edited

Loading

codecov bot commented Jun 8, 2024 •

edited

Loading

matmair commented Jun 17, 2024

SchrodingersGat commented Jun 18, 2024

matmair left a comment

matmair Jun 18, 2024

SchrodingersGat Jun 18, 2024

matmair Jun 18, 2024

matmair Jun 18, 2024

SchrodingersGat Jun 18, 2024

matmair Jun 18, 2024

SchrodingersGat Jun 18, 2024

matmair Jun 18, 2024

matmair commented Jun 18, 2024

SchrodingersGat commented Jun 18, 2024 •

edited

Loading

matmair commented Jun 19, 2024

		if get_global_setting('INVENTREE_INSTANCE_TITLE'):
		return get_global_setting('INVENTREE_INSTANCE')

Single table for file attachments #7420

Single table for file attachments #7420

Conversation

SchrodingersGat commented Jun 8, 2024 • edited Loading

Tasks

Related Issues

netlify bot commented Jun 8, 2024 • edited Loading

✅ Deploy Preview for inventree-web-pui-preview canceled.

codecov bot commented Jun 8, 2024 • edited Loading

Codecov Report

matmair commented Jun 17, 2024

SchrodingersGat commented Jun 18, 2024

matmair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matmair commented Jun 18, 2024

SchrodingersGat commented Jun 18, 2024 • edited Loading

matmair commented Jun 19, 2024

SchrodingersGat commented Jun 8, 2024 •

edited

Loading

netlify bot commented Jun 8, 2024 •

edited

Loading

codecov bot commented Jun 8, 2024 •

edited

Loading

SchrodingersGat commented Jun 18, 2024 •

edited

Loading