Skip to content

[10.0][ADD][web_chatter_paste] - #548

Merged
lasley merged 20 commits into
OCA:10.0from
tarteo:100_add_web_chatter_paste
Mar 21, 2017
Merged

lasley merged 20 commits into
OCA:10.0from
tarteo:100_add_web_chatter_paste

Conversation

@tarteo

@tarteo tarteo commented Feb 3, 2017

Copy link
Copy Markdown
Member

This module makes it easier to upload attachments while composing a message (chatter).

Paste an image (e.g. a screenshot) or drop any file into the composer and it will be automatically added as attachment.

Browser support:

  • Dropping files only works in Chrome.

@JordiBForgeFlow

Copy link
Copy Markdown
Member

Thanks @mreficent @lreficent can you review? It osnfor v10

@tarteo

tarteo commented Feb 3, 2017

Copy link
Copy Markdown
Member Author

Can somebody help me with the tests? I don't know how to make tests for controllers.

@LoisRForgeFlow

Copy link
Copy Markdown
Contributor

Tested on runbot, I can attach both ways but when sending the message it is not actually sent. I don't know if this is a runbot problem though.

@pedrobaeza

Copy link
Copy Markdown
Member

Hi, why this doesn't work on all the browsers (or Firefox mainly)? Is this not working also on mobiles (the paste option, of course)?

@tarteo

tarteo commented Feb 6, 2017

Copy link
Copy Markdown
Member Author

@lreficent This is an issue of runbot. I installed the module on a fresh environment and it worked.

@tarteo

tarteo commented Feb 6, 2017

Copy link
Copy Markdown
Member Author

@pedrobaeza I made a mistake. It actually works for Firefox, but only for Gecko version 50 or higher.

https://developer.mozilla.org/nl/docs/Web/API/DataTransferItem

@LoisRForgeFlow LoisRForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested on local with Firefox and Chromium 👍

@lasley

lasley commented Feb 22, 2017

Copy link
Copy Markdown
Contributor

@tarteo - Regarding the controller tests, what kind of issues were you running into? Looking at this controller, it could be something simple like:

res = ChatterPasteController().upload_attachment(..)
self.assertDictEqual(res, expected)
self.assertTrue(
    self.env['ir.attachment'].browse(res['id']),
)

@tarteo

tarteo commented Feb 22, 2017

Copy link
Copy Markdown
Member Author

@lasley I don't remember anymore... haha. Thanks though! I will make the tests asap.

@tarteo

tarteo commented Feb 27, 2017

Copy link
Copy Markdown
Member Author

@lasley I don't understand why my tests are not working. It works on my development machine.

ERROR: test_controller (odoo.addons.web_chatter_paste.tests.test_web_chatter_paste.TestWebChatterPaste)
Traceback (most recent call last):
File "/home/travis/build/OCA/web/web_chatter_paste/tests/test_web_chatter_paste.py", line 14, in test_controller attachment_obj = self.env['ir.attachment'].with_env(request.env)
File "/home/travis/.local/lib/python2.7/site-packages/werkzeug/local.py", line 343, in __getattr__ return getattr(self._get_current_object(), name)
File "/home/travis/.local/lib/python2.7/site-packages/werkzeug/local.py", line 302, in _get_current_object return self.__local()
File "/home/travis/.local/lib/python2.7/site-packages/werkzeug/local.py", line 135, in _lookup raise RuntimeError('object unbound')
` RuntimeError: object unbound
FAILED
Module web_chatter_paste: 0 failures, 1 errors
At least one test failed when loading the modules.
Modules loaded message not found.

Can you help me please?

@tarteo

tarteo commented Mar 1, 2017

Copy link
Copy Markdown
Member Author

Tests are working locally but not here. Is this a known problem?

@lasley

lasley commented Mar 1, 2017

Copy link
Copy Markdown
Contributor

@tarteo - sorry I missed your last comment. Hrmmm it's working on Runbot too. I honestly have no idea, maybe try a rebase on 10.0? This totally looks like something glitchy, and not something your end - I'm not even able to load the Travis build.
image

tarteo added 4 commits March 3, 2017 12:44
Initial commit

Initial commit

[ADD] Tests

[FIX] Dependencies

[REM] Controller tests

[FIX] Removed unnecessary incompatibility warning

[FIX] Hyperlink to issues

[ADD] Tests

[FIX] Tests

[?] Testing test problem

[?] Testing test problem

[?] Testing test problem
@tarteo

tarteo commented Mar 20, 2017

Copy link
Copy Markdown
Member Author

@lasley I've made it work using 'MagicMock' https://docs.python.org/3/library/unittest.mock.html 😄

'c1gYQAAAAASUVORK5CYII='

request = http.request
http.request = MagicMock(env=self.env)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not correctly unpatching. You should mock using with statements & patching like so:

with mock.patch.object(http, 'request') as request:
    request.env = self.env
    ....

Not doing this will leave the request object as a mock in the http module. This will then affect subsequent tests if they require a request.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh I didn't know that. Thanks I'll fix it.

@tarteo

tarteo commented Mar 21, 2017

Copy link
Copy Markdown
Member Author

@lasley fixed it

@lasley lasley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @tarteo

@lasley
lasley merged commit 0ac4052 into OCA:10.0 Mar 21, 2017
Known issues / Roadmap
======================

* Dropping files only works in Chrome.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this comment outdated now?

@tarteo
tarteo deleted the 100_add_web_chatter_paste branch October 30, 2019 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants