[ADD] business_requirement_deliverable_cost #6

victormmtorres · 2016-06-03T03:48:25Z

File: business_requirement_deliverable_cost/models/business.py:L1-222
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> do you really need the 2 steps?
File: business_requirement_deliverable_cost/models/business.py:L1-222
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> why the logic between the 2 methods is different (previous and this one?)
File: business_requirement_deliverable_cost/views/business.xml:L1-61
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> should be odoo
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Not in 8.0
File: business_requirement_deliverable_cost/views/business.xml:L1-61
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> you can remove this tag
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> This is 8.0, so it's not possible
File: business_requirement_deliverable_cost/views/business.xml:L1-57
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> @victormartinelicocorp My mistake, I forgot tht this s v8.0...
Please replace back to openerp (shame)
File: business_requirement_deliverable_cost/README.rst:L1-184
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Same comment as the other PR about this repetition
File: business_requirement_deliverable_cost/README.rst:L1-184
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Remove empty sections
File: business_requirement_deliverable_cost/openerp.py:L1-29
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> s/Requirements/Requirement (singular)
File: business_requirement_deliverable_cost/models/business.py:L1-218
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Is this really needed? Hasn't got the object already the required ACLs?
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> I guess this is a question more for functional user. These fields has sensible information and for our company is necessary to have different users levels to have restrictions on field level.
@elicoidal?
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> Yes: 2 levels of ACL have been created specifically:
some users (developers) only have access to the time or tasks description
Salesmen can see the sales price but not necessarily the cost of the resources.
You want a PM to be able to have the gross profit of the BR
File: business_requirement_deliverable_cost/models/business.py:L1-218
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> This should have a self.ensure_one() instead the loop.
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Anyway, why don't define this as a computed field?
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> Before, I've never thought to use a computed field to use as an auxiliary field on others calculations but I seems reasonable and safe. @elicoidal @dreispt agree to add a computed field partner_id?
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> Agree: there is no reason why it should be different from header
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> check [ADD] business_requirement_deliverable_cost #6 (comment)
File: business_requirement_deliverable_cost/models/business.py:L1-218
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> This should have a self.ensure_one() instead the loop
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Anyway, why don't define this as a computed field?
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> > Before, I've never thought to use a computed field to use as an auxiliary field on others calculations but I seems reasonable and safe. @elicoidal @dreispt agree to add a computed field partner_id?
same as above @elicoidal @dreispt ?
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> idem
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> > pedrobaeza 9 days ago
This should have a self.ensure_one() instead the loop
ok

dreispt 3 days ago
A computed field can't be overridden manually; a onchange proposed value can.
IMO it's more flexible to keep this defined in an onchange.
Then I'll follow @dreispt recommendations. I also prefer flexibility.
File: business_requirement_deliverable_cost/models/business.py:L1-218
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Don't put plurals on fields names
File: business_requirement_deliverable_cost/models/business.py:L1-218
<img border=0 src='https://avatars.githubusercontent.com/u/1246629?v=3' height=16 width=16'> Can we make this a single line?
General Comment
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> @elicoidal also I add button to Update Pricelist so manager can decide if wants to recalculate again.
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage remained the same at 86.218% when pulling d1a8b6b on victormartinelicocorp:4_business_requirement_deliverable_cost into af25af1 on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage remained the same at 86.218% when pulling 4ae9ed7 on victormartinelicocorp:4_business_requirement_deliverable_cost into af25af1 on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage decreased (-4.4%) to 86.218% when pulling c442ed1 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage decreased (-1.3%) to 89.32% when pulling 9fd81bf on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage increased (+3.2%) to 93.851% when pulling f008d91 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> @elicoidal Please review.
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> 👍 (small nitpicks)
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> I will fix the README file (images+runbot) in a parallel PR
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage increased (+3.2%) to 93.851% when pulling fd8175e on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> @elicoidal
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> 👍
@dreispt @jbeficent @pedrobaeza Thanks for your help
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> @dreispt @pedrobaeza can we merge this one?
<img border=0 src='https://avatars.githubusercontent.com/u/7165771?v=3' height=16 width=16'> Don't put the same icon as the main one. You can use it that one with a "sub-icon" in one corner representing the specific feature this module adds.
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> > Don't put the same icon as the main one. You can use it that one with a "sub-icon" in one corner representing the specific feature this module adds.
@elicoidal ?
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage increased (+3.4%) to 94.006% when pulling f67d0f7 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/2354108?v=3' height=16 width=16'>
Coverage increased (+3.4%) to 94.006% when pulling e5611b3 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> @gurneyalex Could you help with https://runbot.odoo-community.org/runbot/build/3154238 ?
status killed.
cc @elicoidal
<img border=0 src='https://avatars.githubusercontent.com/u/2032807?v=3' height=16 width=16'> @victormartinelicocorp I triggered a rebuild after restarting postgresql. ping me if this does not help.
<img border=0 src='https://avatars.githubusercontent.com/u/7600613?v=3' height=16 width=16'> @pedrobaeza

Don't put the same icon as the main one. You can use it that one with a "sub-icon" in one corner representing the specific feature this module adds.
Yes: non-blocking but I will work on a set of icon for each of the modules
<img border=0 src='https://avatars.githubusercontent.com/u/16461806?v=3' height=16 width=16'> @elicoidal @dreispt @pedrobaeza Could review?

…le_cost Improved the README.rst and icon for business_requirement_deliverable_cost.

…pricelist based on customer. and UnitTest regards refactoring. modified: business_requirement_deliverable_cost/models/business.py modified: business_requirement_deliverable_cost/tests/test_br.py modified: business_requirement_deliverable_cost/views/business.xml

victormmtorres · 2016-08-24T09:18:55Z

@elicoidal also I add button to Update Pricelist so manager can decide if wants to recalculate again.

coveralls · 2016-08-24T09:37:51Z

Coverage remained the same at 86.218% when pulling d1a8b6b on victormartinelicocorp:4_business_requirement_deliverable_cost into af25af1 on OCA:8.0.

coveralls · 2016-08-24T09:37:51Z

Coverage remained the same at 86.218% when pulling d1a8b6b on victormartinelicocorp:4_business_requirement_deliverable_cost into af25af1 on OCA:8.0.

elicoidal · 2016-08-24T09:37:57Z

business_requirement_deliverable_cost/models/business.py

+    @api.onchange('product_id')
+    def product_id_change(self):
+        super(BusinessRequirementResource, self).product_id_change()
+        unit_price = 0


do you really need the 2 steps?

coveralls · 2016-08-24T10:12:38Z

Coverage remained the same at 86.218% when pulling 4ae9ed7 on victormartinelicocorp:4_business_requirement_deliverable_cost into af25af1 on OCA:8.0.

coveralls · 2016-08-24T10:12:38Z

Coverage remained the same at 86.218% when pulling 4ae9ed7 on victormartinelicocorp:4_business_requirement_deliverable_cost into af25af1 on OCA:8.0.

…pricelist based on customer. and UnitTest regards refactoring. modified: business_requirement_deliverable_cost/models/business.py modified: business_requirement_deliverable_cost/tests/test_br.py modified: business_requirement_deliverable_cost/views/business.xml

…not passed. modified: business_requirement_deliverable_cost/__openerp__.py modified: business_requirement_deliverable_cost/models/business.py

coveralls · 2016-08-26T05:04:07Z

Coverage decreased (-4.4%) to 86.218% when pulling c442ed1 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.

…UT on specific functions. Fix others comments made by GitHub reviewers. modified: business_requirement_deliverable_cost/models/business.py modified: business_requirement_deliverable_cost/tests/test_br.py

coveralls · 2016-08-29T08:17:29Z

Coverage decreased (-1.3%) to 89.32% when pulling 9fd81bf on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.

…e Coverall. modified: business_requirement_deliverable_cost/tests/test_br.py

coveralls · 2016-08-29T08:36:25Z

Coverage increased (+3.2%) to 93.851% when pulling f008d91 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.

victormmtorres · 2016-08-29T08:45:40Z

@elicoidal Please review.

elicoidal · 2016-08-29T10:26:20Z

business_requirement_deliverable_cost/views/business.xml

@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="utf-8"?>
+<openerp>


should be odoo

pedrobaeza · 2016-09-09T07:34:28Z

Don't put the same icon as the main one. You can use it that one with a "sub-icon" in one corner representing the specific feature this module adds.

victormmtorres · 2016-09-09T08:37:27Z

Don't put the same icon as the main one. You can use it that one with a "sub-icon" in one corner representing the specific feature this module adds.

@elicoidal ?

Remove useless introduction from README.rst

coveralls · 2016-09-12T11:19:42Z

Coverage increased (+3.4%) to 94.006% when pulling f67d0f7 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.

…ble_cost 4 business requirement deliverable cost

coveralls · 2016-09-12T11:42:23Z

Coverage increased (+3.4%) to 94.006% when pulling e5611b3 on victormartinelicocorp:4_business_requirement_deliverable_cost into 8442e3f on OCA:8.0.

victormmtorres · 2016-09-13T05:58:39Z

@gurneyalex Could you help with https://runbot.odoo-community.org/runbot/build/3154238 ?
status killed.

cc @elicoidal

gurneyalex · 2016-09-13T13:22:38Z

@victormartinelicocorp I triggered a rebuild after restarting postgresql. ping me if this does not help.

…nt_deliverable_cost rebase 4 business requirement deliverable cost

dreispt · 2016-09-15T14:49:19Z

business_requirement_deliverable_cost/models/business.py

+    @api.multi
+    @api.depends(
+        'deliverable_lines'
+    )


Can we make this a single line?

dreispt · 2016-09-15T14:49:49Z

business_requirement_deliverable_cost/models/business.py

+    @api.multi
+    @api.depends(
+        'deliverable_lines'
+    )


elicoidal · 2016-09-16T00:16:46Z

@pedrobaeza

Don't put the same icon as the main one. You can use it that one with a "sub-icon" in one corner representing the specific feature this module adds.

Yes: non-blocking but I will work on a set of icon for each of the modules

…ble_cost 4 business requirement deliverable cost

…itHub regards decorator multi. modified: business_requirement_deliverable_cost/models/business.py modified: business_requirement_deliverable_cost/tests/test_br.py modified: business_requirement_deliverable_cost/views/business.xml

…rds python style. modified: business_requirement_deliverable_cost/models/business.py

victormmtorres · 2016-09-18T12:15:54Z

@elicoidal @dreispt @pedrobaeza Could review?

elicoidal · 2016-09-18T12:22:28Z

👍

elicoidal · 2016-09-30T06:26:36Z

@victormartinelicocorp is this one clean to merge?

victormmtorres · 2016-09-30T06:43:07Z

Yes @elicoidal

elicoidal · 2016-09-30T07:25:08Z

@dreispt @pedrobaeza can we move forward?

elicoidal · 2016-10-04T08:45:36Z

Merging for the code sprint

…case-field [IMP]business_requirement: add test case field

victor and others added 7 commits June 3, 2016 11:44

[WIP] business_requirement_deliverable_cost

6733304

Improved the icon and README introduction

37961d6

Improved README introduction and icon

68108e9

Improved README introduction

1294368

add module name in the image path in rst

81a0be2

Merge pull request #5 from elicoidal/4_business_requirement_deliverab…

e4637c0

…le_cost Improved the README.rst and icon for business_requirement_deliverable_cost.

elicoidal reviewed Aug 24, 2016
View reviewed changes

Corrected README file (figure+runbot)

60ef6c6

victor and others added 7 commits August 26, 2016 12:55

[WIP] business_requirement_deliverable_cost

63f5289

Improved the icon and README introduction

4274695

Improved README introduction and icon

27e305a

Improved README introduction

4f80743

add module name in the image path in rst

62c5d18

[FIX] business_requirement_deliverable_cost: Fix Travis. Test flake8 …

c442ed1

…not passed. modified: business_requirement_deliverable_cost/__openerp__.py modified: business_requirement_deliverable_cost/models/business.py

victormmtorres force-pushed the 4_business_requirement_deliverable_cost branch from 4ae9ed7 to c442ed1 Compare August 26, 2016 04:58

[FIX] business_requirement_deliverable_cost: Fix Coveralls by adding …

9fd81bf

…UT on specific functions. Fix others comments made by GitHub reviewers. modified: business_requirement_deliverable_cost/models/business.py modified: business_requirement_deliverable_cost/tests/test_br.py

[FIX] business_requirement_deliverable_cost: Add test left to increas…

f008d91

…e Coverall. modified: business_requirement_deliverable_cost/tests/test_br.py

elicoidal reviewed Aug 29, 2016
View reviewed changes

elicoidal and others added 4 commits September 11, 2016 12:34

Update README.rst

37fd756

Delete bus_req_module_diag.png

b7e64af

Delete bus_req_tree.png

11581ee

Merge pull request #15 from elicoidal/patch-3

f67d0f7

Remove useless introduction from README.rst

Merge pull request #21 from elicoidal/4_business_requirement_delivera…

e5611b3

…ble_cost 4 business requirement deliverable cost

elicoidal added 2 commits September 14, 2016 19:27

Merge pull request #3 from victormartinelicocorp/4_business_requireme…

4d2624d

…nt_deliverable_cost rebase 4 business requirement deliverable cost

fixed __openerp__.py (images/description/category)

a5a30cc

sbidoul removed the Needs Review label Sep 15, 2016

dreispt reviewed Sep 15, 2016

View reviewed changes

elicoidal changed the title ~~[WIP] business_requirement_deliverable_cost~~ business_requirement_deliverable_cost Sep 15, 2016

elicoidal changed the title ~~business_requirement_deliverable_cost~~ [ADD] business_requirement_deliverable_cost Sep 16, 2016

Victor Martin added 3 commits September 18, 2016 12:27

Merge pull request #29 from elicoidal/4_business_requirement_delivera…

c739a32

…ble_cost 4 business requirement deliverable cost

[FIX] business_requirement_deliverable_cost: Fix others comments rega…

82ad62e

…rds python style. modified: business_requirement_deliverable_cost/models/business.py

elicoidal added the needs review label Sep 18, 2016

elicoidal merged commit 5ce0696 into OCA:8.0 Oct 4, 2016

elicoidal pushed a commit that referenced this pull request Dec 28, 2016

Merge pull request #6 from RawEvan/9.0-business-requirement-add-test-…

7744b86

…case-field [IMP]business_requirement: add test case field

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] business_requirement_deliverable_cost #6

[ADD] business_requirement_deliverable_cost #6

victormmtorres commented Jun 3, 2016 •

edited by dreispt

victormmtorres commented Aug 24, 2016

coveralls commented Aug 24, 2016

coveralls commented Aug 24, 2016

elicoidal Aug 24, 2016

coveralls commented Aug 24, 2016

coveralls commented Aug 24, 2016

coveralls commented Aug 26, 2016 •

edited

coveralls commented Aug 29, 2016 •

edited

coveralls commented Aug 29, 2016 •

edited

victormmtorres commented Aug 29, 2016

elicoidal Aug 29, 2016

pedrobaeza Sep 9, 2016

pedrobaeza commented Sep 9, 2016

victormmtorres commented Sep 9, 2016

coveralls commented Sep 12, 2016 •

edited

coveralls commented Sep 12, 2016 •

edited

victormmtorres commented Sep 13, 2016

gurneyalex commented Sep 13, 2016

dreispt Sep 15, 2016

dreispt Sep 15, 2016

elicoidal commented Sep 16, 2016

victormmtorres commented Sep 18, 2016

elicoidal commented Sep 18, 2016

elicoidal commented Sep 30, 2016

victormmtorres commented Sep 30, 2016

elicoidal commented Sep 30, 2016

elicoidal commented Oct 4, 2016

		@@ -0,0 +1,60 @@
		<?xml version="1.0" encoding="utf-8"?>
		<openerp>

[ADD] business_requirement_deliverable_cost #6

[ADD] business_requirement_deliverable_cost #6

Conversation

victormmtorres commented Jun 3, 2016 • edited by dreispt

victormmtorres commented Aug 24, 2016

coveralls commented Aug 24, 2016

coveralls commented Aug 24, 2016

elicoidal Aug 24, 2016

Choose a reason for hiding this comment

coveralls commented Aug 24, 2016

coveralls commented Aug 24, 2016

coveralls commented Aug 26, 2016 • edited

coveralls commented Aug 29, 2016 • edited

coveralls commented Aug 29, 2016 • edited

victormmtorres commented Aug 29, 2016

elicoidal Aug 29, 2016

Choose a reason for hiding this comment

pedrobaeza Sep 9, 2016

Choose a reason for hiding this comment

pedrobaeza commented Sep 9, 2016

victormmtorres commented Sep 9, 2016

coveralls commented Sep 12, 2016 • edited

coveralls commented Sep 12, 2016 • edited

victormmtorres commented Sep 13, 2016

gurneyalex commented Sep 13, 2016

dreispt Sep 15, 2016

Choose a reason for hiding this comment

dreispt Sep 15, 2016

Choose a reason for hiding this comment

elicoidal commented Sep 16, 2016

victormmtorres commented Sep 18, 2016

elicoidal commented Sep 18, 2016

elicoidal commented Sep 30, 2016

victormmtorres commented Sep 30, 2016

elicoidal commented Sep 30, 2016

elicoidal commented Oct 4, 2016

victormmtorres commented Jun 3, 2016 •

edited by dreispt

coveralls commented Aug 26, 2016 •

edited

coveralls commented Aug 29, 2016 •

edited

coveralls commented Aug 29, 2016 •

edited

coveralls commented Sep 12, 2016 •

edited

coveralls commented Sep 12, 2016 •

edited