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

[Fix][8.0] Fix the way pricelists are selected when a template_id is set as rule of pricelist #100

Merged
merged 2 commits into from
Feb 23, 2018

Conversation

llacroix
Copy link

@llacroix llacroix commented May 6, 2016

Added a method that help matching which pricelist can be used.

It can be inherited which allow other modules to extend the way the
pricelists are chosen.

Also a rule in the while loop could be removed as it duplicated
one of the tests when chosing the available pricelists.

Fix the issue here:
#90

@llacroix llacroix changed the title Added a method to match rules. [Fix][8.0] Fix the way pricelists are selected when a template_id is set as rule of pricelist May 6, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 41.309% when pulling 94ca89e on llacroix:8_fix_pos_pricelist_template into b66ec78 on OCA:8.0.

// Check category
cond = cond && (
item.categ_id === false ||
categ_ids.indexOf(item.categ_id[0]) !== -1
Copy link
Member

Choose a reason for hiding this comment

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

Where is categ_ids defined?

Copy link
Author

Choose a reason for hiding this comment

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

Ah this seems to be missing when I moved some code to that method. I'll fix that.

@StefanRijnhart
Copy link
Member

Thanks for the fix! I added some small discussion points to the code.

@llacroix
Copy link
Author

I fear it might not be that simple. I remember it was working with pos_pricelist alone, but apparently with pos_template there are still some problems :(

@llacroix
Copy link
Author

Ah non forget my previous comment, I just forgot to move some code in the function I added. The problem I had is something completely different.

@llacroix
Copy link
Author

llacroix commented Nov 7, 2016

Any news about this patch?

@rafaelbn
Copy link
Member

rafaelbn commented Dec 1, 2016

Hi @ismaelcj @jcarlosmontoya @AdilHoumadi , have you experimented this issue with pos_pricelist? Maybe you could take a look here. Thanks!

@rafaelbn
Copy link
Member

rafaelbn commented Dec 1, 2016

Hi @krupesh-weboffice, this PR resolves your issue #90, could you test it please? Thanks!

@rafaelbn
Copy link
Member

Hi @llacroix , there have been some changes in travis. Please could you rebase this PR in order to check it again? Thanks

@llacroix
Copy link
Author

I guess I can do that. A bit later today.

@legalsylvain
Copy link
Contributor

@llacroix : could you rebase ?
@krupesh-weboffice : could you test this patch ?

thanks.

It can be inherited which allow other modules to extend the way the
pricelists are chosen.

Also a rule in the while loop could be removed as it duplicated
one of the tests when chosing the available pricelists.
@llacroix llacroix force-pushed the 8_fix_pos_pricelist_template branch from 496330b to 3aab34a Compare April 18, 2017 10:30
@llacroix
Copy link
Author

Rebased

@pedrobaeza
Copy link
Member

Is this OK? Or should it be closed?

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Code review, no test.

@pedrobaeza
Copy link
Member

Merging this after a while with one verified reviewer and green status.

@pedrobaeza pedrobaeza merged commit e8b5508 into OCA:8.0 Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants