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][MIG][BR001139][T23297]Migrated Project Completion report from v8 to v10 #194

Open
wants to merge 15 commits into
base: 10.0
Choose a base branch
from

Conversation

YogeshMahera-SerpentCS
Copy link

  • Migrated Project Completion Report from v8 to v10

  • hr.analytic.timesheet --> account.analytic.line

  • project.task.work --> account.analytic.line

  • project_timesheet --> hr_timesheet

@elicoidal @seb-elico @victormartinelicocorp

Copy link
Contributor

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

needs tests

activity_stage_id = fields.Many2one(
'project.task.type', 'Stage',
readonly=True, help="Activity Stage")
# FIXME if BR resource UoM is not hours, `qty` needs to be converted
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you take care of this too?

Choose a reason for hiding this comment

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

Do I need to convert the Day(s) uom only or should convert Unit(s) as well into Hour(s)?
@elicoidal @seb-elico

Copy link
Contributor

Choose a reason for hiding this comment

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

@elicoidal it's a tricky one, better to not fix it for now since it might have a huge impact on the performance

@YogeshMahera-SerpentCS please leave the FIXME, we'll fix it later

Choose a reason for hiding this comment

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

Sure @seb-elico

@@ -1,26 +1,28 @@
<?xml version="1.0" encoding="utf-8"?>
<openerp>
<odoo>
<data>
Copy link
Contributor

Choose a reason for hiding this comment

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

remove if noupdate is absent

Copy link
Contributor

@seb-elico seb-elico left a comment

Choose a reason for hiding this comment

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

Missing UT, other than that LGTM

@YogeshMahera-SerpentCS
Copy link
Author

@seb-elico This module related report so Is that require UT?

@seb-elico
Copy link
Contributor

@yogesh Yes, we need to make sure that the computation done by the report is correct

@seb-elico
Copy link
Contributor

@YogeshMahera-SerpentCS please cherry-pick a5b65c4 (bug fix of the v8 report)

@YogeshMahera-SerpentCS
Copy link
Author

YogeshMahera-SerpentCS commented Oct 18, 2017

@seb-elico I have added basic test-cases but not complete whole SQL query computation.
Could you please check it.

@seb-elico
Copy link
Contributor

@YogeshMahera-SerpentCS
Copy link
Author

@seb-elico Could you please review it

@YogeshMahera-SerpentCS
Copy link
Author

@seb-elico Could you please review it

@seb-elico
Copy link
Contributor

@YogeshMahera-SerpentCS @elicoidal Pending merge of #187 to build Travis

@seb-elico
Copy link
Contributor

@YogeshMahera-SerpentCS Instead of creating all the UT data in the UT, you should use the demo data created by this module and its parent modules. For instance, there are multiple BRs/Projects/Tasks/TMS in the demo data. You could use them for the UT. Also, you can create some additional demo data for this module and use it in the UT. If unclear, check with @victormartinelicocorp

@elicoidal
Copy link
Contributor

Travis is red @YogeshMahera-SerpentCS @seb-elico

@YogeshMahera-SerpentCS
Copy link
Author

@seb-elico Travis is red due to

ParseError: "External ID not found in the system: portal.partner_demo_portal" while parsing /home/travis/OCB-10.0/addons/project_issue/data/project_issue_demo.xml:36, near

<field name="partner_id" ref="portal.partner_demo_portal"/>

@YogeshMahera-SerpentCS YogeshMahera-SerpentCS force-pushed the 10.0-MIG-project_completion_report branch from 9b3e4e6 to 8e37daa Compare November 6, 2017 06:40
'user_id': fields.many2one('res.users', 'User', readonly=True),
'project_id': fields.many2one(
'project.project', 'Project', readonly=True),
'project_state': fields.char(
Copy link
Contributor

Choose a reason for hiding this comment

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

@seb-elico
Copy link
Contributor

@lonelysun Please make sure all latest commits of v8 report are included in this PR 👍

@elicoidal
Copy link
Contributor

@seb-elico I tend to think we should close this PR

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.

3 participants