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

[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" #266

Merged
merged 4 commits into from
Nov 22, 2017
Merged

[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" #266

merged 4 commits into from
Nov 22, 2017

Conversation

JoJoJoJoJoJoJo
Copy link
Collaborator

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo commented Nov 14, 2017 •
edited
Loading

add field terms_and_conditions and amend the printout

@oca-clabot
Copy link

Hey @JoJoJoJoJoJoJo, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

elicoidal
elicoidal suggested changes Nov 15, 2017
View reviewed changes
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.

LGTM: some details

Good job!

business_requirement/models/business.py Outdated
@@ -67,6 +67,12 @@ def _get_default_company(self):
states={'draft': [('readonly', False)],
'confirmed': [('readonly', False)]}
)
terms_and_conditions = fields.Html(
'Terms and Conditions',
read_only=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

why readonly?
Shouldnot we add copy=true as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

default copy=True

business_requirement/views/business_view.xml Outdated
@@ -63,6 +63,9 @@
<group string="Test Case">
<field name="test_case" nolabel="1" default_focus="0"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

remove default_focus="0"

@elicoidal elicoidal added this to the 10.0 milestone Nov 15, 2017
@JoJoJoJoJoJoJo
Copy link
Collaborator Author

@elicoidal
changes done by your request.

elicoidal
elicoidal suggested changes Nov 15, 2017
View reviewed changes
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.

almost perfect!
On the functional tests:

  • Only thing missing: the field should not be printed in the printout if it is empty.

@JoJoJoJoJoJoJo
Copy link
Collaborator Author

JoJoJoJoJoJoJo commented Nov 16, 2017 •
edited
Loading

Only thing missing: the field should not be printed in the printout if it is empty.

@elicoidal
Any details about the test condition? I tested locally and it works normally

update:
I've found the situation, it seems when you create a BR, <p><br></p> will be added in the field, it's invisible but not empty

@elicoidal
Copy link
Contributor

@JoJoJoJoJoJoJo I understand then it is LGTM!
Good job :)

elicoidal
elicoidal approved these changes Nov 16, 2017
View reviewed changes
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.

LGTM

@elicoidal
Copy link
Contributor

cc @victormartinelicocorp

victormmtorres
victormmtorres suggested changes Nov 21, 2017
View reviewed changes
Copy link
Collaborator

@victormmtorres victormmtorres left a comment

Choose a reason for hiding this comment

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

bump module version, the rest LGTM

@JoJoJoJoJoJoJo
Copy link
Collaborator Author

@victormartinelicocorp
version bumped
cc @elicoidal @lonelysun

@elicoidal elicoidal merged commit a264778 into OCA:10.0 Nov 22, 2017
@elicoidal elicoidal mentioned this pull request Jan 5, 2018
Closed
@oca-clabot
Copy link

Hey @JoJoJoJoJoJoJo,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

ruter-lyu pushed a commit to ruter-lyu/business-requirement that referenced this pull request Mar 15, 2019
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
045a02e 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 4, 2019
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
d17f34e 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
pedrobaeza pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 21, 2019
@pedrobaeza
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
d71540b 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
victormmtorres pushed a commit to Tecnativa/business-requirement that referenced this pull request Jun 26, 2019
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
e85cfa0 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
Tardo pushed a commit to Tecnativa/business-requirement that referenced this pull request Jan 24, 2020
@Tardo
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
77cf981 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
victoralmau pushed a commit to Tecnativa/business-requirement that referenced this pull request Oct 14, 2021
@victoralmau
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
efa8b26 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
victoralmau pushed a commit to Tecnativa/business-requirement that referenced this pull request Oct 14, 2021
@victoralmau
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
840c0a8 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
MosabWadea pushed a commit to MosabWadea/business-requirement that referenced this pull request Mar 5, 2022
@MosabWadea
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
d836d4b 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
AntoniRomera pushed a commit to AntoniRomera/business-requirement that referenced this pull request Jul 4, 2023
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
9727486 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
NachoAlesLopez pushed a commit to NachoAlesLopez/business-requirement that referenced this pull request Apr 28, 2024
@NachoAlesLopez
[IMP] [10.0] BR001208-T24350: Add field "Terms and Conditions" (OCA#266)
fbc3751 
* [IMP] add terms and conditions field to the model and update the reprts

* [FIX] spell issue and remove default_focus=0 in test case

* [FIX] issue of empty field printed in the report

* [FIX] upgrade version number
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elicoidal elicoidal elicoidal approved these changes

@lonelysun lonelysun lonelysun approved these changes

@victormmtorres victormmtorres victormmtorres approved these changes

Assignees
No one assigned
Labels
approved needs review
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JoJoJoJoJoJoJo @oca-clabot @elicoidal @lonelysun @victormmtorres