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

Fix : Don't parse onchange resullt when returning False instead of nothing #61

Merged
merged 1 commit into from
Aug 23, 2021
Merged

Fix : Don't parse onchange resullt when returning False instead of nothing #61

merged 1 commit into from
Aug 23, 2021

Conversation

flotho
Copy link
Member

@flotho flotho commented Aug 5, 2021

In case of onchange method calling method that retrun possible false values https://github.com/OCA/OCB/blob/10.0/addons/account/models/partner.py#L162 the syntax value[0] failed because of the structure fo the dictionnary :
values = {'fiscal_position_id': False}
This patch prevent to pass in those lines

@OCA-git-bot
Copy link
Contributor

Hi @flotho! Thank you very much for this contribution. As the addon you are improving does not have a declared maintainer, I take the opportunity to mention that you can consider adopting it. To do so, please read the maintainer role description, and, if interested, create a pull request to add your GitHub login to the maintainers key of the addon manifest.

@flotho
Copy link
Member Author

flotho commented Aug 5, 2021 •
edited
Loading

@pedrobaeza , how is it possible to propagate this on later release?
Any specific command to add ?

@pedrobaeza pedrobaeza added this to the 10.0 milestone Aug 5, 2021
@pedrobaeza
Copy link
Member

Do you mean forward-port it?

@flotho
Copy link
Member Author

flotho commented Aug 5, 2021

Do you mean forward-port it?

yes

@pedrobaeza
Copy link
Member

Manually cherry-picking the commit and proposing/committing (if you are maintainer) it, or try to use this tool:

OCA/maintainer-tools#504

@flotho
Copy link
Member Author

flotho commented Aug 9, 2021

Hi @pedrobaeza

Regarding runbot, what can I do?

BTW, I'm waiting everything green to squash my commits ;-)

@pedrobaeza
Copy link
Member

Forcing rebuild can lead to get a proper one. I have launched it to see.

@flotho
Copy link
Member Author

flotho commented Aug 11, 2021

Hi @pedrobaeza thanks for your help.
I can't see anything in the logs and I'm not able to restart the build
image

sorry for those boring technical questions ;-)

@pedrobaeza
Copy link
Member

No need to launch again runbot. The logs are there: https://runbot2-3.odoo-community.org/runbot/static/build/3497520-61-5b4a29/logs/job_20_test_all.txt

and these are the warnings:

2021-08-10 18:41:48,753 184 WARNING openerp_test odoo.models: exception.rule.create() includes unknown fields: rule_group
2021-08-10 18:41:48,760 184 WARNING openerp_test odoo.models: exception.rule.create() includes unknown fields: rule_group
2021-08-10 18:41:48,765 184 WARNING openerp_test odoo.models: exception.rule.create() includes unknown fields: rule_group

@flotho
Copy link
Member Author

flotho commented Aug 11, 2021

Thanks @pedrobaeza ,

In the end, it looks like I already have this issue OCA/sale-workflow#1058

@flotho
Fix : onchange that False instead of nothing
5523a20 
In case of onchange method calling method that retrun possible false values https://github.com/OCA/OCB/blob/10.0/addons/account/models/partner.py#L162 the syntax value[0] failed because of the structure fo the dictionnary :
values = {'fiscal_position_id': False}
This patch prevent to pass in those lines

Update sale_order_onchange.py

fix pylint/flake

Delete rule_group from data

to deal with OCA/sale-workflow#1058

Update ecommerce_data.xml

(facepalm)
@flotho
Copy link
Member Author

flotho commented Aug 16, 2021

Hi there,

everything looks green, could we merge ?

@flotho
Copy link
Member Author

flotho commented Aug 18, 2021

any chance to be merged @pedrobaeza ?
Regards

@pedrobaeza
Copy link
Member

This requires at least 2 reviews from qualified people.

@flotho
Copy link
Member Author

flotho commented Aug 19, 2021

This requires at least 2 reviews from qualified people.

(facepalm) you're right, holidays were too short!

how can I identify maintainers of this repo?

@pedrobaeza
Copy link
Member

Check previous committers of the module, but it also helps and it's fair that you review other PRs in exchange.

@flotho
Copy link
Member Author

flotho commented Aug 21, 2021

Hi @lmignon @guewen

found your contribution in the history.
Any review possible here ?

Regards

lmignon
lmignon approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

LGTM (code review)

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@lmignon
Copy link
Contributor

lmignon commented Aug 23, 2021

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 10.0-ocabot-merge-pr-61-by-lmignon-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b319920 into OCA:10.0 Aug 23, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 5733a43. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmignon lmignon lmignon approved these changes

@guewen guewen guewen approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.
5 participants
@flotho @OCA-git-bot @pedrobaeza @lmignon @guewen