[WIP] [MIG] product_multi_image: Module totally redone from old product_images by pedrobaeza · Pull Request #57 · OCA/product-attribute

pedrobaeza · 2015-03-12T08:10:25Z

This is still a work in progress.

Refactorized:

Take into account templates and products
Code for creating and updating
Views
All!

New planned features:

Three possible sources: URL, file or DB.
Possibility to share images between different products.
New kanban view
Others not yet thought.

guewen · 2015-03-12T12:47:54Z

product_multi_image/models/product_image.py

It is unlikely to happen but there is a small chance that 2 calls are done at the same time, one call will raise an OSError. I think the line could be replaced by the following because we do not mind if it does no longer exist.

try: os.remove(self.path) except OSError: pass

Also, I think the file should be deleter after the call to super, so if something raise in super, the file won't be deleted.

You're right, thanks.

bealdav · 2015-04-09T20:20:03Z

@sebastienbeau, take a look, you have done job on this matter

flotho · 2015-08-20T12:27:38Z

Hi,

I don't understand the problem with runbot.
The issue seems to come from the dataset that has already 2 codes identical.
Any chance to be explained what is the process to change this?

LeartS · 2015-08-21T09:00:14Z

I forced a rebuild, let's see what happens.

LeartS · 2015-08-21T09:29:36Z

Well, seems to be an error due to the module product_sequence. The strange thing is that module has some code specifically to handle this case: https://github.com/OCA/product-attribute/blob/8.0/product_sequence/models/product_product.py#L26

Maybe it never gets called? I see the module partner_firstname uses _install_<module-name> to do something similar.

Those part dispappeared from the PR and was present in the OCA repo. This code reintegrate the missing part OCA#57 (comment)

flotho · 2015-08-21T12:25:25Z

Hi @LeartS

Thanks for your report.
Actually the branch from @pedrobaeza missed this part of code.
So I made this pedrobaeza#1
Once it will be pulled we'll be able to rerun the test.

Thanks

LeartS · 2015-08-21T12:42:48Z

@flotho I've just noticed that this PR is older than that code, @pedrobaeza should just rebase this branch and it should fix itself.

flotho · 2015-08-21T12:53:59Z

@LeartS
Ok sorry, I didn't checked well your comment, I did it for @pedrobaeza, we only have to wait now.

pedrobaeza · 2015-08-21T17:14:43Z

Branch rebased. Let's see CIs' result.

flotho · 2015-08-25T07:18:24Z

Hi,

I don't understand the reason of the runbot built failure. It talks about mrp, yet there is no mrp in this PR.

flotho · 2015-09-11T20:36:04Z

Any chance to have some odoo core updates and rerun this one?

Those part dispappeared from the PR and was present in the OCA repo. This code reintegrate the missing part OCA#57 (comment)

rafaelbn · 2015-10-27T16:22:34Z

cc @yajo @sergio-incaser @carlos-incaser

yajo · 2015-10-29T15:17:13Z

product_multi_image/README.rst

This is missing some important parts such as usage instructions, try on runbot, report issues, etc.

yajo · 2015-10-29T15:20:36Z

product_multi_image/models/product_image.py

Please prefix with _ following guidelines.

yajo · 2015-10-29T15:36:26Z

Full code review done for now, with some remarks as you see. You did 828 additions and 448 deletions, some tests would be appreciated. Also please fix runbot to start functional review. Thanks!

pedrobaeza · 2015-10-29T15:41:35Z

@yajo, that was done a lot of time ago. My skills have improved since then 😉 I'll check your comments later

elicoidal · 2015-10-29T22:57:49Z

product_multi_image/models/product_image.py

a URL or an URL?

yajo · 2015-10-30T15:16:56Z

I'm thinking of another approach, let's see what you think.

What about putting all the gallery-related stuff in an abstract model in a separate module, and then inherit it in products and add the views?

I'm sure with time more and more models will benefit of having a gallery (events, blog...). This way we'd reuse the abstract one and repeat less code.

pedrobaeza · 2015-10-30T15:19:09Z

Yeah, it's a very good idea. We need them 2 modules: base_multi_image, with all the stuff that handles the possible sources, cache... and then inherit in product_multi_image, event_multi_image... Are you going to do it?

yajo · 2015-10-30T16:25:37Z

Let's ask @rafaelbn.

rafaelbn · 2015-10-30T21:05:31Z

I agree, let's go. Please, talking about images they must be stored outside data base. How to solve this?

pedrobaeza · 2015-10-31T10:47:05Z

For now, there's 3 possibilities:

Store in database
Put a path of the file server. This is located outside of the DB, but the file handling is made outside Odoo.
Put a URL. This is for avoiding making verbatim copies of images, if you already have them on another hosting (like in Magento, Prestashop...).

We can add a fourth option that is "stored as a attachment".

yajo · 2015-11-02T16:32:02Z

I'm forking this to develop. Close for now if you wish.

pedrobaeza · 2015-11-02T16:32:53Z

OK, eager to see the new approach

pedrobaeza mentioned this pull request Mar 12, 2015

Migration of product_images to 8.0 #42

Closed

pedrobaeza added the work in progress label Mar 12, 2015

pedrobaeza mentioned this pull request Mar 12, 2015

[WIP] [MIG] Migrated Prestashop connector to v8 OCA/connector-prestashop#11

Closed

3 tasks

guewen reviewed Mar 12, 2015
View reviewed changes

pedrobaeza force-pushed the 8.0-product_images branch from b5bcd11 to 8998fe6 Compare March 13, 2015 00:49

pedrobaeza force-pushed the 8.0-product_images branch from 8998fe6 to fb8a91c Compare March 20, 2015 09:14

pedrobaeza mentioned this pull request Mar 23, 2015

8.0 add addresses mgmt OCA/stock-logistics-transport#6

Merged

1 task

lukebranch mentioned this pull request Apr 30, 2015

Download images OdooCommunityWidgets/website_multi_image#12

Open

flotho added a commit to MindAndGo/product-attribute that referenced this pull request Aug 21, 2015

Readd missing controls on duplicate default_code

7244e25

Those part dispappeared from the PR and was present in the OCA repo. This code reintegrate the missing part OCA#57 (comment)

flotho mentioned this pull request Aug 21, 2015

[FIX]Readd missing controls on duplicate default_code pedrobaeza/product-attribute#1

Closed

pedrobaeza added 2 commits August 21, 2015 19:14

[MIG] product_multi_image: Module totally redone from old product_images

d2183b7

[FIX] product_multi_image: PEP8

f5dc106

pedrobaeza force-pushed the 8.0-product_images branch from b48ad34 to f5dc106 Compare August 21, 2015 17:14

flotho added a commit to MindAndGo/product-attribute that referenced this pull request Sep 15, 2015

Readd missing controls on duplicate default_code

c10a197

Those part dispappeared from the PR and was present in the OCA repo. This code reintegrate the missing part OCA#57 (comment)

flotho mentioned this pull request Sep 15, 2015

Readd missing controls on duplicate default_code pedrobaeza/product-attribute#3

Closed

yajo reviewed Oct 29, 2015
View reviewed changes

product_multi_image/models/product_image.py

Copy link
Copy Markdown

Member

yajo Oct 29, 2015

Choose a reason for hiding this comment

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

Please prefix with _ following guidelines.

elicoidal reviewed Oct 29, 2015
View reviewed changes

product_multi_image/models/product_image.py

Copy link
Copy Markdown

elicoidal Oct 29, 2015

Choose a reason for hiding this comment

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

a URL or an URL?

pedrobaeza mentioned this pull request Nov 2, 2015

[ADD] new module website_multi_image OCA/e-commerce#70

Closed

pedrobaeza closed this Nov 2, 2015

yvaucher removed the work in progress label Nov 2, 2015

yajo mentioned this pull request Nov 19, 2015

[8.0][base_multi_image][product_multi_image] Multi images for products split in abstract and concrete models #112

Closed

atchuthan mentioned this pull request Mar 17, 2016

Migration to version 9.0 #100

Closed

37 tasks

Uh oh!

Conversation

pedrobaeza commented Mar 12, 2015

Uh oh!

guewen Mar 12, 2015

Choose a reason for hiding this comment

Uh oh!

guewen Mar 12, 2015

Choose a reason for hiding this comment

Uh oh!

pedrobaeza Mar 12, 2015

Choose a reason for hiding this comment

Uh oh!

bealdav commented Apr 9, 2015

Uh oh!

flotho commented Aug 20, 2015

Uh oh!

LeartS commented Aug 21, 2015

Uh oh!

LeartS commented Aug 21, 2015

Uh oh!

flotho commented Aug 21, 2015

Uh oh!

LeartS commented Aug 21, 2015

Uh oh!

flotho commented Aug 21, 2015

Uh oh!

pedrobaeza commented Aug 21, 2015

Uh oh!

flotho commented Aug 25, 2015

Uh oh!

flotho commented Sep 11, 2015

Uh oh!

rafaelbn commented Oct 27, 2015

Uh oh!

yajo Oct 29, 2015

Choose a reason for hiding this comment

Uh oh!

yajo Oct 29, 2015

Choose a reason for hiding this comment

Uh oh!

yajo commented Oct 29, 2015

Uh oh!

pedrobaeza commented Oct 29, 2015

Uh oh!

elicoidal Oct 29, 2015

Choose a reason for hiding this comment

Uh oh!

yajo commented Oct 30, 2015

Uh oh!

pedrobaeza commented Oct 30, 2015

Uh oh!

yajo commented Oct 30, 2015

Uh oh!

rafaelbn commented Oct 30, 2015

Uh oh!

pedrobaeza commented Oct 31, 2015

Uh oh!

yajo commented Nov 2, 2015

Uh oh!

pedrobaeza commented Nov 2, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants