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

10.0 ir attachment url ecommerce #775

Conversation

Projects
None yet
5 participants
@KolushovAlexandr
Copy link
Member

commented Jul 5, 2019

No description provided.

Show resolved Hide resolved ir_attachment_s3/models/ir_attachment.py Outdated
Show resolved Hide resolved ir_attachment_s3/models/ir_attachment.py Outdated

RafiZz and others added some commits Dec 17, 2018

@KolushovAlexandr KolushovAlexandr force-pushed the KolushovAlexandr:10.0-ir_attachment_url_ecommerce branch from 7173dd2 to 71c2ab8 Jul 5, 2019

@@ -17,6 +17,10 @@ env:
global:
- VERSION="10.0" TESTS="0" LINT_CHECK="0" UNIT_TEST="0"
- PYLINT_ODOO_JSLINTRC="/home/travis/maintainer-quality-tools/travis/cfg/.jslintrc"
- secure: "SoW2Blve4jUApyF5UfM5NvGQ6ZV4OH6wMUGbcTemx+dMqNZTyTlUDxmVOFgau7M/igcgmmdYcsxmWKR0n40VSaAvtBKSdL7J3Ad7xVBpC1zq6LLrrDmZUL1pCWQUpl1znxcq72H+VmnllgdewU1703cVce2wUWmByGjEEV/VnIqjUfXfzh28ZL4hMr/Kkstr2MCBHMP2n/LZJqBCa5CWyH2U+gxDhQ3zNKudZbHgdkLWYhGSlnVsqTxsxTLaIcXOdDIesT5JYHByQhqVNAuURZuT9RFq9+GWAcFbc4luGhkRhfoZGa7VOAwEwdYNTIip01xgiChL2GfvBEoi8sPKVY4UNKhkWoU6j7IxVVvXnVVlbflpNhQkLnsZD6Yd4r3ZnD+3o8oOb9H69yOOABlsBb90wwE+XMB2fikftXTNfeKeRLKiAGorTe4jwQoqN9Mbd/aTIXzYeFOJu+zwjoFbp8wY0lGYQm4e7VFkf5cRr7YQ0kYLu8LbwhIGmJVppJhA7tsWncLkHxAZ56Up1+nkw8W4bsetkWuNw+Y5XOIXjfmEJ7nR3kSL3vDHKU5C7M674KVtjmCjabc7Rgs/+iXlf0WFVXlXUOOr/2PHfO3kq8zL3PcrehfHPR70wxPFgu7kw9Nl835Ve4J2yuG01NSJvzNr+pLk9mkVNhLoOiKonmI=" # TRANSIFEX_PASSWORD

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

Delete transfix please. We don't use it anymore

resized_attachment = \
image_variant_attachment._get_resized_from_cache(w, h) or \
image_variant_attachment._set_resized_to_cache(w, h, field=field)
attachment = resized_attachment.resized_attachment_id

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

copy-pasted code (difference in field=field is not counted). Consider to make a single method instead (event if that method just calls those two methods)


        resized_attachment = \
            attachment._get_resized_from_cache(width, height) or \
            attachment._set_resized_to_cache(width, height)
`1.2.1`
-------

- **FIX:** The Odoo System parameters saving from the Environment variables removed because then we cannot change parameters through environment variables

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

This is not the only update



        # TODO: if model=="product.product" and field in ('image', 'image_small', 'image_medium')
        # we need to make similar trick, because those fields are computed resizes of image_variant
        # with sizes 64*64, 128*128, 1024*1024

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

The description says "We removed feature X, because then we cannot use feature X"

image_variant_attachment._set_resized_to_cache(w, h, field=field)
attachment = resized_attachment.resized_attachment_id

if not attachment and model != 'ir.attachment':
attachment = env['ir.http'].find_field_attachment(env, model, field, obj)

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

I think there is a way to refactor it in way where you can deduplicate this line of code:

attachment = env['ir.http']._find_field_attachment(env, model, field, obj.id)

# with sizes 64*64, 128*128, 1024*1024
if not (res.status_code == 301 and (width or height)):
is_product_product_image = model == 'product.product' and field in ('image', 'image_small', 'image_medium')
if not (res.status_code == 301 and (width or height)) and not is_product_product_image:
return res

# * check that it's image on s3

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

Please extend this algorithm description according to your updates

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

And probably move it to the method description

def content_image(....):
"""* check that ...
* ....
"""
self.assertTrue(product_product_image_medium_attachment)
self.assertTrue(product_product_image_small_attachment)

self.assertEqual(redirected_image_url, product_product_image_attachment.url)

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

How does it works if you don't have image case in the inverse method? Fix the tests first.



            if attach.res_field == 'image_medium':
                value = tools.image_resize_image(base64_source=attach.datas, size=(128, 128),
                                                 encoding='base64', filetype='PNG')
            elif attach.res_field == 'image_small':
                value = tools.image_resize_image(base64_source=attach.datas, size=(64, 64),
                                                 encoding='base64', filetype='PNG')
@route()
def content_image(self, xmlid=None, model='ir.attachment', id=None, field='datas', filename_field='datas_fname', unique=None, filename=None, mimetype=None, download=None, width=0, height=0):
res = super(BinaryExtended, self).content_image(xmlid, model, id, field, filename_field, unique, filename, mimetype, download, width, height)
return res

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

Why do we change here?

if value and save_option != 'url':
r = requests.get(value)
base64source = base64.b64encode(r.content)
super(Binary, self).write(records, base64source)

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

Does that mean, that module can be used to save files from url (probably after changing parameter to any other value, say "filestore"). If so, you need to cover this in the docs.
If this code has other purposes too, add a comment here. I guess it pass file content to super in order to handle in s3 module, for example

@@ -115,6 +115,7 @@ def binary_content(cls, xmlid=None, model='ir.attachment', id=None, field='datas
if att:
content = att.url
status = 301
mimetype = att.mimetype

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

Why do we redefine mimetype variable passed to the method? Can original mimetype has not a Non wrong value?

required=True,
)

def set_ir_attachment_save_option(self):

This comment has been minimized.

Copy link
@yelizariev

yelizariev Jul 12, 2019

Member

Method is not used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.