Skip to content

Conversation

@Reza98Sh
Copy link
Contributor

@Reza98Sh Reza98Sh commented Oct 4, 2025

This pull request introduces a major refactoring of the image management system, transitioning from a distributed approach to a unified, single-table architecture. By centralizing image storage into a new InvenTreeImage model, this change simplifies the database schema, reduces data duplication, and provides a more robust and extensible foundation for handling images across the application.

This work is a continuation of the efforts and discussions from the previous pull request (#10069)

Key Architectural Changes & Features

  • Unified InvenTreeImage Model: All uploaded images are now stored in a single InvenTreeImage table (previously named UploadedImage). This model uses a Generic Foreign Key to associate images with various other models (e.g., Part, Company), making the system highly extensible for future use cases and plugins.
  • Duplicate Prevention: A sha256 hash is now calculated and stored for each uploaded image. The system uses this hash to prevent identical images from being stored multiple times, saving significant storage space.
  • Modernized Path Handling: All file path operations have been updated from os.path to pathlib.Path for improved cross-platform compatibility and code readability.
  • Automatic Cleanup: A custom CustomStdImageField has been implemented, which automatically deletes image files from the storage when the corresponding database record is removed, preventing orphaned files.
  • Reusable Serialization: A new InvenTreeImageSerializerMixin provides image_url and thumbnail_url fields, ensuring consistent and efficient image serialization across different API endpoints while mitigating N+1 query problems.

Tasks Completed

  • Created a new, unified table for all images (InvenTreeImage).
  • Implemented a robust data migration script to move all existing Part and Company images to the new table.
  • Removed old, redundant image tables after successful migration.
  • Implemented a mixin-style architecture using Generic Foreign Keys for associating images with other models.
  • Exposed the new InvenTreeImage model through a dedicated API namespace.
  • Restored and improved previous image functionalities (e.g., thumbnails) in the new model.
  • Updated the Part and Company UI to use the new image management system, including an image carousel for multiple images.
  • Resolved all merge conflicts with the master branch.
  • Added comprehensive unit tests for the new API, including tests for image migration (TestLegacyImageMigration).
  • Optimized all new and modified API endpoints to prevent N+1 query issues.
  • Updated the script for creating the test database.
  • Update test database creation script

New API Endpoints

A new API namespace for images has been introduced:

Method Endpoint Description
GET /api/image/ List all images
POST /api/image/ Upload a new image
GET /api/image/<pk>/ Retrieve a specific image
PATCH /api/image/<pk>/ Update a specific image
DELETE /api/image/<pk>/ Delete a specific image
GET /api/image/thumbs/ Retrieve image thumbnails

Addressing Previous Feedback

Based on the review of PR #10069, the following changes were made:

  • Model Naming: UploadedImage was renamed to InvenTreeImage for clarity.
  • Generic Relations: The association mechanism was rebuilt using GenericForeignKey instead of a custom implementation, improving integration possibilities for plugins.
  • API Consistency: Thumbnail URLs are now exposed directly via the serializers, maintaining consistency with previous API behavior and avoiding breaking changes where possible.
  • Code Cleanup: All commented-out code has been removed in favor of clean, intentional changes.
  • Test Structure: Generic tests for the InvenTreeImage model are now located in common/, while model-specific tests (e.g., for Part) remain in their respective app directories.

This PR is now complete, with all tests passing and functionality fully implemented. It is ready for a final review.


Related Issues

Closes #9788
Supersedes #10069

@netlify
Copy link

netlify bot commented Oct 4, 2025

Deploy Preview for inventree-web-pui-preview ready!

Name Link
🔨 Latest commit 329ded9
🔍 Latest deploy log https://app.netlify.com/projects/inventree-web-pui-preview/deploys/6910ee754e2384000834f4b0
😎 Deploy Preview https://deploy-preview-10484--inventree-web-pui-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 93 (🔴 down 2 from production)
Accessibility: 81 (no change from production)
Best Practices: 100 (no change from production)
SEO: 78 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@Reza98Sh Reza98Sh marked this pull request as draft October 4, 2025 13:40
@codecov
Copy link

codecov bot commented Oct 4, 2025

Codecov Report

❌ Patch coverage is 95.86984% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.09%. Comparing base (726e852) to head (329ded9).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10484      +/-   ##
==========================================
+ Coverage   87.92%   88.09%   +0.16%     
==========================================
  Files        1278     1286       +8     
  Lines       57447    58045     +598     
  Branches     2005     1993      -12     
==========================================
+ Hits        50512    51133     +621     
+ Misses       6437     6416      -21     
+ Partials      498      496       -2     
Flag Coverage Δ
backend 89.32% <74.21%> (-0.22%) ⬇️
migrations 42.72% <56.44%> (+0.28%) ⬆️

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

Components Coverage Δ
Backend Apps 91.99% <100.00%> (+0.05%) ⬆️
Backend General 93.94% <95.64%> (+0.27%) ⬆️
Frontend 70.50% <ø> (+0.09%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@matmair
Copy link
Contributor

matmair commented Oct 4, 2025

@Reza98Sh some CI tests are not passing, please address that before we start reviews

@matmair
Copy link
Contributor

matmair commented Oct 4, 2025

Could we change the API references from thumbnail_url to thumbnail and image_url to image? That would reduce the breaking changes in the API

@matmair matmair added enhancement This is an suggested enhancement or new feature api Relates to the API labels Oct 4, 2025
@matmair matmair added this to the 1.1.0 milestone Oct 4, 2025
@Reza98Sh
Copy link
Contributor Author

Reza98Sh commented Oct 9, 2025

Could we change the API references from thumbnail_url to thumbnail and image_url to image? That would reduce the breaking changes in the API

for part that have multiple images

"images": [
        {
        "pk": 0,
        "primary": true,
        "image": "http://example.com/",
        "thumbnail": "string"
        }
],
"image_url": "string",
"thumbnail_url": "string",

but for company image that only have one image:

"image": {
            "pk": 1,
            "primary": true,
            "image": "/media/images/company_3_img_OAbJ7MY.jpg",
            "thumbnail": "/media/images/company_3_img_OAbJ7MY.thumbnail.jpg"
        },
"image_url": "string",
"thumbnail_url": "string",

What do you think about this change?

@matmair
Copy link
Contributor

matmair commented Oct 12, 2025

Maybe I have not communicated the goal good enough: current users of the API have request with the fields thumbnail and image - with this change they will have to change their calls to thumbnail_url and image_url. Why do we need to break the API for those users?

@Reza98Sh
Copy link
Contributor Author

I noticed the confusion came from turning image and thumbnail from simple URL fields into objects after moving images to a separate table.

To avoid naming conflicts and keep the API backward-compatible, I’ll remove the embedded image object from Part and Company serializers. Instead, these endpoints will once again return image and thumbnail as URL fields — just like before.

They will again return image and thumbnail as URLs, while detailed image data can be fetched from the new /api/image/ endpoint.

@matmair
Copy link
Contributor

matmair commented Oct 12, 2025

You can extend the fields as much as you want, but renaming old fields without a good reason is an issue.

@matmair
Copy link
Contributor

matmair commented Oct 22, 2025

@Reza98Sh are you ok with me implementing some of the open comments? Would like to get this over the line

@Reza98Sh
Copy link
Contributor Author

I’ve been quite busy over the past couple of weeks.

I’ve reviewed the context, and your review makes perfect sense. I will work on implementing these changes and plan to push the new code within the next few days.

Thanks you!

@Reza98Sh
Copy link
Contributor Author

Reza98Sh commented Oct 23, 2025

@matmair
Excuse me.
Should I also add unit test in the front end ?
Could you give me a few hint how do I write test in front end?

@Reza98Sh Reza98Sh force-pushed the feature/multiplePartImage#t2 branch from 6b78e03 to 41b3f36 Compare October 24, 2025 08:39
@Reza98Sh
Copy link
Contributor Author

I’ve added several tests for the migrations. However, the code coverage report is flagging the test file itself (src/backend/InvenTree/common/test_migrations.py) for low coverage !
I don't know how I can increase code coverage

@matmair matmair added the full-run Always do a full QC CI run label Oct 24, 2025
@Reza98Sh Reza98Sh force-pushed the feature/multiplePartImage#t2 branch from a519343 to 8e49133 Compare October 24, 2025 12:23
@matmair
Copy link
Contributor

matmair commented Oct 25, 2025

@Reza98Sh touching up areas of the frontend that are changed without coverage would be great. We use Playwright for frontend tests, the framework has a pretty great vscode extension that I would recommend and is well-supported by copilot and other AI agents if you run into issues. The tests are in src/frontend/tests

@matmair
Copy link
Contributor

matmair commented Oct 25, 2025

Some test are not passing, which is why the files are getting flagged; solving the test not passing should adress the code coverage issues or make them much less significant

@matmair
Copy link
Contributor

matmair commented Oct 25, 2025

Most of the failing test seem to related to the renaming / removal / change of default behavior (None if nothing is set, not a blank image reference) on the image and thumbnail fields; which is what I was warning about above

we can either change the name in the tests or of the fields to address that

@Reza98Sh Reza98Sh force-pushed the feature/multiplePartImage#t2 branch from 5e5aefd to e6ff0b2 Compare October 27, 2025 07:47
@Reza98Sh Reza98Sh force-pushed the feature/multiplePartImage#t2 branch from db41564 to a0ced07 Compare October 31, 2025 16:06
@Reza98Sh
Copy link
Contributor Author

@matmair

I’ve fixed the failed tests related to the database. Now I need some help with the remaining CI failures.

Could you guide me on how to pass these tests?

QC / Style [Typecheck]
QC / Tests - API Schema Documentation
QC / Tests - inventree-python

I couldn't find test files related to QC / Tests - inventree-python

@matmair
Copy link
Contributor

matmair commented Nov 1, 2025

Typecheck and API Schema Documentation can be ignored - the first was an issue with external infra; the API docs passed but the change is so large that it times out

inventree-python is an integration test against https://github.com/inventree/inventree-python
If these tests fail it is highly preferable to fix the API behavior instead of the tests. Consumers will not update their client version immediately

@Reza98Sh
Copy link
Contributor Author

Reza98Sh commented Nov 1, 2025

The response schema for the part and company list/detail endpoint remains unchanged. However, for adding and deleting images for parts, the old API approach can no longer be used because it may have multiple images.

What is your suggestion?

@Reza98Sh Reza98Sh force-pushed the feature/multiplePartImage#t2 branch from 5ea92f5 to 329ded9 Compare November 9, 2025 19:41
@Reza98Sh Reza98Sh marked this pull request as ready for review November 10, 2025 03:43
@Reza98Sh
Copy link
Contributor Author

Reza98Sh commented Nov 10, 2025

@matmair
@SchrodingersGat
I think it’s ready for the next round of review

@Reza98Sh
Copy link
Contributor Author

If it needs further changes or refinement, please feel free to let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Relates to the API enhancement This is an suggested enhancement or new feature full-run Always do a full QC CI run refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Multiple Part Pictures

2 participants