-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
[8.0]-[BR1030]-[ADD/IMP][Reference field improvements] #208
[8.0]-[BR1030]-[ADD/IMP][Reference field improvements] #208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@victormartinelicocorp @seb-elico
@@ -40,7 +40,7 @@ def _get_default_company(self): | |||
states={'draft': [('readonly', False)]} | |||
) | |||
ref = fields.Char( | |||
'Reference', | |||
'WBS', | |||
required=False, | |||
readonly=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why r/o?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I haven't changed it was already there.
But yes, I think read-only should not be there, should I remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure as there are some ACL based on status. I dont know if it has an impact so dont modify it here.
@@ -269,7 +270,8 @@ def name_get(self): | |||
result = [] | |||
for br in self: | |||
if br.ref: | |||
formatted_name = u'[{}] {}'.format(br.ref, br.description) | |||
formatted_name = u'[{}][{}] {}'.format(br.ref, br.name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@darshan-serpent What is the point of have twice the br.name? Perhaps you forgot add other field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you are right. I think It should be like, right?
formatted_name = u'[{}] {}'.format(br.ref, br.name, br.description)
Will do the needful, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure here but if the requirement is fulfilled, this is fine by me.
I want: [WBS][name] Description or [name] Description (if WBS empty)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@elicoidal @victormartinelicocorp Tested on runbot, works as expected. The test is made with 2 BRs, one with WBS and other without it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
@victormartinelicocorp please review again |
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
* [8.0][BR1030][ADD][Reference field improvements] * [IMP]Improved code
Issue:- #184
Improved string Reference to "WBS"
Improved name_get method
Added origin field with text type
@elicoidal @victormartinelicocorp