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

Mig mgmtsystem review to v. 10.0 #171

Merged
merged 1 commit into from
Jun 16, 2017

Conversation

eugen-don
Copy link
Member

@eugen-don eugen-don commented Mar 14, 2017

Mig mgmtsystem review to v. 10.0
depends:

@max3903 max3903 modified the milestone: 10.0 Mar 18, 2017
@pedrobaeza pedrobaeza mentioned this pull request Mar 18, 2017
31 tasks
@eugen-don eugen-don force-pushed the MIG-mgmtsystem_review-to-V.-10.0 branch 3 times, most recently from e3ebb7f to c1d714a Compare March 22, 2017 08:45
@eugen-don eugen-don changed the title Mig mgmtsystem review to v. 10.0 WIP Mig mgmtsystem review to v. 10.0 Mar 23, 2017
@eugen-don eugen-don force-pushed the MIG-mgmtsystem_review-to-V.-10.0 branch from c1d714a to c327f4a Compare March 25, 2017 15:14
@eugen-don eugen-don mentioned this pull request Mar 26, 2017
3 tasks
@eugen-don eugen-don changed the title WIP Mig mgmtsystem review to v. 10.0 Mig mgmtsystem review to v. 10.0 Mar 26, 2017
@eugen-don eugen-don force-pushed the MIG-mgmtsystem_review-to-V.-10.0 branch from 228be61 to 4f51ff5 Compare March 27, 2017 06:30
from . import (
mgmtsystem_review,
mgmtsystem_review_line,
)
Copy link
Member

Choose a reason for hiding this comment

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

The previous format if OK (and more concise). No point in changing it.

@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-

from openerp.tests import common
from odoo.tests import common
from datetime import datetime
Copy link
Member

Choose a reason for hiding this comment

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

Question for odoo experts:

could be better to replace ?:

from datetime import datetime
...
datetime.now()

to :

from odoo import fields
...
fields.datetime.now()

@eugen-don eugen-don force-pushed the MIG-mgmtsystem_review-to-V.-10.0 branch from 8c4727f to bdb9070 Compare June 13, 2017 07:24
@eugen-don
Copy link
Member Author

eugen-don commented Jun 13, 2017

Hi @max3903 @dreispt
i had problems reverting or lets say rewriting the first commit message, luckily figured out how git reset command works.

Do you think this looks ok now?

@max3903
Copy link
Member

max3903 commented Jun 13, 2017

@eugen-don Please look at the report attached:
Review report (9).pdf

I can't see the title and the footer.

Copy link
Member

@max3903 max3903 left a comment

Choose a reason for hiding this comment

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

Report needs fixing.

@eugen-don
Copy link
Member Author

@max3903 how did you print this report i cant find the menuitem?

@max3903
Copy link
Member

max3903 commented Jun 13, 2017

@eugen-don I created a review and went to the print menu.

@eugen-don
Copy link
Member Author

@max3903

reports are OK, check it out one more time, paper format wasnt set accordingly.

@eugen-don
Copy link
Member Author

ping @angelmoya

@angelmoya
Copy link
Member

Sorry @eugen-don this week I have a lot of work and no time to check this, maybe friday or next week.

Please @acm1pt-colorado check this

Copy link

@ljimenezsidoo ljimenezsidoo left a comment

Choose a reason for hiding this comment

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

Hi @eugen-don tested on runbot, looks great

@angelmoya
Copy link
Member

angelmoya commented Jun 14, 2017

On runbot I create "AngelMoyaReview" and get this error when I try to print

Traceback (most recent call last):
  File "/home/odoo/odoo-10.0/addons/report/controllers/main.py", line 96, in report_download
    response = self.report_routes(reportname, docids=docids, converter='pdf')
  File "/.repo_requirements/odoo/odoo/http.py", line 504, in response_wrap
    response = f(*args, **kw)
  File "/home/odoo/odoo-10.0/addons/report/controllers/main.py", line 45, in report_routes
    pdf = report_obj.with_context(context).get_pdf(docids, reportname, data=data)
  File "/home/odoo/odoo-10.0/addons/report/models/report.py", line 178, in get_pdf
    html = self.with_context(context).get_html(docids, report_name, data=data)
  File "/home/odoo/odoo-10.0/addons/report/models/report.py", line 144, in get_html
    return self.render(report.report_name, docargs)
  File "/home/odoo/odoo-10.0/addons/report/models/report.py", line 120, in render
    return view_obj.render_template(template, values)
  File "/.repo_requirements/odoo/odoo/addons/base/ir/ir_ui_view.py", line 1037, in render_template
    return self.browse(self.get_view_id(template)).render(values, engine)
  File "/home/odoo/odoo-10.0/addons/website/models/ir_ui_view.py", line 110, in render
    return super(View, self).render(values, engine=engine)
  File "/home/odoo/odoo-10.0/addons/web_editor/models/ir_ui_view.py", line 26, in render
    return super(IrUiView, self).render(values=values, engine=engine)
  File "/.repo_requirements/odoo/odoo/addons/base/ir/ir_ui_view.py", line 1057, in render
    return self.env[engine].render(self.id, qcontext)
  File "/.repo_requirements/odoo/odoo/addons/base/ir/ir_qweb/ir_qweb.py", line 53, in render
    return super(IrQWeb, self).render(id_or_xml_id, values=values, **context)
  File "/.repo_requirements/odoo/odoo/addons/base/ir/ir_qweb/qweb.py", line 248, in render
    self.compile(template, options)(self, body.append, values or {})
  File "/.repo_requirements/odoo/odoo/addons/base/ir/ir_qweb/qweb.py", line 317, in _compiled_fn
    raise e
QWebException: tuple index out of range
Traceback (most recent call last):
  File "/.repo_requirements/odoo/odoo/addons/base/ir/ir_qweb/qweb.py", line 315, in _compiled_fn
    return compiled(self, append, values, options, log)
  File "<template>", line 1, in template_mgmtsystem_review_review_report_1735
  File "<template>", line 4, in body_call_content_1734
  File "<template>", line 74, in foreach_1722
  File "/.repo_requirements/odoo/odoo/models.py", line 5215, in __getitem__
    return self._browse((self._ids[key],), self.env)
IndexError: tuple index out of range

Error to render compiling AST
IndexError: tuple index out of range
Template: mgmtsystem_review.review_report
Path: /templates/t/t/div/div[4]/div[3]/t[1]/ul/li[1]/span
Node: <span t-field="response.user_input_line_ids[2].value_text"/>

@eugen-don
Copy link
Member Author

eugen-don commented Jun 14, 2017

@angelmoya
The reson for the error is that the your review contains survey answers which arent completed yet. See in your review there are survey answers that are partly complete. Try to first complete the survey answers and then add the survey answers to the review again.

@angelmoya
Copy link
Member

Nice @eugen-don , would be better to show this information in warning to prevent this error.

@eugen-don
Copy link
Member Author

@angelmoya
indeed thanks for the good idea.

@dreispt @max3903
This behaviour is the same in all versions.

I see two ways of correcting this:

  1. add a filter to the Survey Answers field in the view definition to only show complete answers
  2. edit the template to check if the surveys answer is completed and only print the answer if it is completed.

Since the first option is easier to code and i assume that including incomplete or empty answers in the modules data and reports provides no benefits, id like to stick to the first option.

What do you guys think?

@max3903
Copy link
Member

max3903 commented Jun 14, 2017

@eugen-don Yes, first option makes more sense.

@eugen-don
Copy link
Member Author

@dreispt

could you merge?

@max3903
Copy link
Member

max3903 commented Jun 16, 2017

@eugen-don Have you added the filter on the survey answer?

@eugen-don
Copy link
Member Author

eugen-don commented Jun 16, 2017

@max3903 no since its not part of the migration, its an improvement or fix and goes into a different PR

since the bug is present in versions 8.0 and 9.0 ill need to fix those first...

but i see no problems on adding a fix right in the PR of the migration of the module.

@max3903 max3903 merged commit 4cae0ac into OCA:10.0 Jun 16, 2017
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.

5 participants