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.

Sign up for GitHub

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

Jump to bottom

Add a new field and change the domain filter #44

Merged
merged 2 commits into from
Nov 14, 2015
Merged

Add a new field and change the domain filter #44

merged 2 commits into from
Nov 14, 2015

Conversation

ehdem
Copy link

@ehdem ehdem commented Mar 11, 2015

The new field function allows to get a filter with the model and mode_id. The result is used to update the domain in the js.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.65%) to 34.15% when pulling 7a70fde on savoirfairelinux:7.0_document_multiple_record_add_field into 26d6c6b on OCA:7.0.

@bwrsandman
Copy link

👎 This breaks the login page when debug mode is disactivated.

Try a new database and just installing this module.

@ehdem
Copy link
Author

ehdem commented Mar 12, 2015

@bwrsandman : Ok, I try this

@ehdem
Copy link
Author

ehdem commented Mar 12, 2015

@bwrsandman: I created a new database and just installed this module document_multiple_record, and I didn't get any error.
What's the issue?

@bwrsandman
Copy link

Did you try:

  1. new database
  2. install the module
  3. go to the login page with debug disactivated (nothing after http://SERVERURL/)

@ehdem
Copy link
Author

ehdem commented Mar 12, 2015

@bwrsandman : I saw the issue and I fixed it.

@ehdem ehdem force-pushed the 7.0_document_multiple_record_add_field branch from a48b8b3 to 172e7d8 Compare March 12, 2015 16:24
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.65%) to 34.15% when pulling 172e7d8 on savoirfairelinux:7.0_document_multiple_record_add_field into 26d6c6b on OCA:7.0.

@ehdem ehdem force-pushed the 7.0_document_multiple_record_add_field branch 2 times, most recently from a372fb4 to 39fb4fb Compare March 17, 2015 17:06
@coveralls
Copy link

Coverage Status

Coverage increased (+9.11%) to 43.9% when pulling 39fb4fb on savoirfairelinux:7.0_document_multiple_record_add_field into 26d6c6b on OCA:7.0.

@ehdem ehdem force-pushed the 7.0_document_multiple_record_add_field branch 2 times, most recently from af05c35 to b9d8bef Compare March 17, 2015 20:39
@coveralls
Copy link

Coverage Status

Coverage increased (+10.88%) to 45.67% when pulling af05c35 on savoirfairelinux:7.0_document_multiple_record_add_field into 26d6c6b on OCA:7.0.

@ehdem ehdem force-pushed the 7.0_document_multiple_record_add_field branch from b9d8bef to ac7be2e Compare March 18, 2015 16:00
@bwrsandman
Copy link

👍

bwrsandman
bwrsandman reviewed Mar 23, 2015
View reviewed changes
document_multiple_records/static/src/js/document.js
@@ -70,8 +75,8 @@ var _t = instance.web._t,
this.on_attachments_loaded([]);
}
else {
var dom = [ ['attachment_document_ids.res_model', '=', dataset.model], ['attachment_document_ids.res_id', '=', model_id], ['type', 'in', ['binary', 'url']] ];
var ds = new instance.web.DataSetSearch(this, 'ir.attachment', dataset.get_context(), dom);
var dom = [ ['related_document', '=', true] ];
Copy link

Choose a reason for hiding this comment

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

If you did var dom = [ ['related_document', '=', [dataset.model, model_id]] ]; you wouldn't need to lose context.

In _get_related_model_documents, you could get these from args

Copy link
Author

Choose a reason for hiding this comment

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

@bwrsandman : Your expression "var dom = [ ['related_document', '=', [dataset.model, model_id]] ]; " is incorrect, related_document is a boolean and [dataset.model, model_id]] is a list.
I just add a filter to list attachments where related_document field is true.

@max3903 max3903 added this to the 7.0 milestone Apr 5, 2015
@yvaucher
Copy link
Member

I set as needs fixing as there are some remarks from @bwrsandman

@ehdem ehdem force-pushed the 7.0_document_multiple_record_add_field branch from ac7be2e to 4258b58 Compare May 7, 2015 18:05
@ehdem
Copy link
Author

ehdem commented May 7, 2015

Can't access the log from https://runbot.odoo-community.org/runbot/build/2946193

@yvaucher
Copy link
Member

yvaucher commented May 8, 2015

@ehdem try to fix first the logs of travis https://travis-ci.org/OCA/knowledge/builds/61657752
A test is failing

bwrsandman
bwrsandman reviewed May 8, 2015
View reviewed changes
document_multiple_records/document.py

ir_attachment_ids = self.search(
cr, uid, [
('attachment_document_ids.id', 'in', ir_attachment_doc_ids)])
Copy link

Choose a reason for hiding this comment

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

context not passed

@ehdem ehdem force-pushed the 7.0_document_multiple_record_add_field branch 2 times, most recently from e04a03e to 3ee3924 Compare May 8, 2015 13:54
@ehdem
[UPD]Change the filter and add the context
9366ebf 
Add a context to keep the model and model_id.
Change the domain filter with the new field

[ADD] Add a new field function

This new function allows to get the related documents with
the model and the model_id.
We add this new field because the domain in the view or in the js
doesn't match the wanted result

[UPD] Add missing comma

[ADD] Add unittests
@max3903 max3903 force-pushed the 7.0_document_multiple_record_add_field branch from 3ee3924 to 9366ebf Compare November 14, 2015 17:27
@max3903
Copy link
Sponsor Member

max3903 commented Nov 14, 2015

👍

max3903 pushed a commit that referenced this pull request Nov 14, 2015
Merge pull request #44 from savoirfairelinux/7.0_document_multiple_re…
6a4752b 
…cord_add_field

Add a new field and change the domain filter
@max3903 max3903 merged commit 6a4752b into OCA:7.0 Nov 14, 2015
@max3903 max3903 deleted the 7.0_document_multiple_record_add_field branch November 14, 2015 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ehdem @coveralls @bwrsandman @yvaucher @max3903