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

[ADD] stock_lot_multi_image: New module #2238

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

GabbasovDinar
Copy link
Member

This module enables the addition of multiple images for each stock lot.

Task: 4241

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more a usage rather than configuration. Because it's not the module configuration but a general module workflow.
Please add a screenshot or screenshots for better UX. They should be no more than 800px width.

@@ -0,0 +1 @@
This module enables the addition of multiple images for each stock lot.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module allows to add multiple images to a stock lot.

@@ -0,0 +1,3 @@
Sometimes it is necessary to have photos of a particular stock lot (item) in addition to the product-level photos (for product.template or product.variant).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes it is necessary to have pictures of a specific product that is tracked by serial numbers.
For example you manage a stock of used laptops where each model is a product variant and each laptop is an item with a unique serial number. And you would like to have photos of each specific laptop, not just the pictures of the model from the official website.
However by default in Odoo you can add pictures only to a product template or a product variant.

@GabbasovDinar GabbasovDinar force-pushed the 16.0-t4241-stock_lot_multi_image-add branch 3 times, most recently from 5b4ceb5 to 088e325 Compare January 17, 2025 11:15
Copy link
Member

@ivs-cetmix ivs-cetmix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review LGTM

Copy link

@Aldeigja Aldeigja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional LGTM

@rousseldenis rousseldenis added this to the 16.0 milestone Jan 17, 2025
"stock",
"base_multi_image",
],
# "images": ["static/description/banner.png"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove?

_name = "stock.lot"
_inherit = ["stock.lot", "base_multi_image.owner"]

image = fields.Image(compute="_compute_image")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also use image.mixin to have the option to store the main image under multiple size, then use image_1920 instead of image as defined here? it maybe useful if someone want to display a thumbnail of the main image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoangtrann I agree, @GabbasovDinar please add dependency on the image.mixin too.

@GabbasovDinar GabbasovDinar force-pushed the 16.0-t4241-stock_lot_multi_image-add branch 3 times, most recently from 320b333 to e17418f Compare January 20, 2025 06:26
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

for lot in self:
lot.image_1920 = (
lot.image_ids
and lot.with_context(bin_size=False).image_ids[0].image_1920
Copy link
Contributor

@rousseldenis rousseldenis Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and lot.with_context(bin_size=False).image_ids[0].image_1920
fields.first(lot.with_context(bin_size=False).image_ids).image_1920

will do the job alone

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rousseldenis thanks! fixed

@GabbasovDinar GabbasovDinar force-pushed the 16.0-t4241-stock_lot_multi_image-add branch from e17418f to 78ebc9b Compare January 22, 2025 14:04
Compute main image of lots
"""
for lot in self:
lot.image_1920 = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arf, I wanted to say the whole expression:

Suggested change
lot.image_1920 = (
lot.image_1920 = fields.first(lot.with_context(bin_size=False).image_ids).image_1920

That's it. if image_ids is void, image_1920 will be False

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

This module enables the addition of multiple images for each stock lot.

Task: 4241
@GabbasovDinar GabbasovDinar force-pushed the 16.0-t4241-stock_lot_multi_image-add branch from 78ebc9b to 8bc43dc Compare January 23, 2025 05:44
@ivs-cetmix
Copy link
Member

Hi @rousseldenis ! Would appreciate your final review and .... merge (if possible of course) 😄

Copy link
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GabbasovDinar @ivs-cetmix I can encourage you to check this module and all the related ones that allows to use the storage you want (S3, ...) https://github.com/OCA/storage/tree/16.0/fs_base_multi_image

@rousseldenis
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2238-by-rousseldenis-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1f9417d into OCA:16.0 Jan 28, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at f2155bb. Thanks a lot for contributing to OCA. ❤️

@ivs-cetmix ivs-cetmix deleted the 16.0-t4241-stock_lot_multi_image-add branch January 28, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants