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

7.0 add sale credit note #121

Closed
wants to merge 19 commits into from
Closed

Conversation

bguillot
Copy link
Contributor

@bguillot bguillot commented Mar 5, 2015

Hello,

This PR adds the module sale_credit_note.

This module allows the user to set a part of a refund as credit note. This credit note is then available on the sale order to reduce the amount that the customer has to pay.
This is also a base module for a module that synchronize credit memo on Magento (the PR is not created yet).

It depends on the PR OCA/e-commerce#38 that creates a hook for the field residual on sale order.

The inheritance of the sale order view that adds the credit lines is ugly, but I needed to put the lines on the same group as the sale total group. I am open to suggestions, and if it worth it I can maybe make a PR on the addons to add a new group.

Thanks for you reviews

@bguillot
Copy link
Contributor Author

bguillot commented Mar 6, 2015

It seems that Travis fails because of the dependency on the module sale_payment_method.
I don't know how to make it pass, the module is in e-commerce branch.

@gurneyalex gurneyalex added this to the 7.0 milestone Mar 12, 2015
@gurneyalex
Copy link
Member

@bguillot you need to add a line in .travis.yml to pull the new dependency.

@bguillot
Copy link
Contributor Author

Thanks for the review, and the advice.

Travis is green now !

class credit_line(orm.Model):
_name = "credit.line"

_columns = {
Copy link

Choose a reason for hiding this comment

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

Please, split each model to a seperate file with the name of the model as file name. It will be easier to maintain. Also, you ommited _description here.

@ghost
Copy link

ghost commented Mar 18, 2015

@bguillot Thanks for the contribution. Could you rename your models with "sale." at the beginning of the models you created (both Models and Transients)? This way, it will be easier to maintain. credit.line could be something else not related to the sales. For exemple, it could be related to accounting. This is just my opinion, but I think the namespace a the beginning of a table name is important.

@oca-clabot
Copy link

Hey @bguillot, 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

@ghost
Copy link

ghost commented Apr 2, 2015

@bguillot Thanks for the change 👍

@gurneyalex
Copy link
Member

@bguillot i've added your github login in the OCA database. This should calm the CLA bot.

@bguillot
Copy link
Contributor Author

bguillot commented Apr 2, 2015

Thank you @gurneyalex , I was a bit confuse about that because I signed the CLA.

Thanks @dufresnedavid for the review ;)

About the runbot, should I put the dependency somewhere as for travis ?

@oca-clabot
Copy link

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

Appreciation of efforts,
OCA CLAbot

@gurneyalex
Copy link
Member

@bguillot we have no support for this yet, and it is still manual.

@gurneyalex
Copy link
Member

@bguillot: would it not make sense to move sale_payment_method from e-commerce to sale-workflow? I'm reluctant to have this circular dependency between the two repositories. Users of e-commerce should not be affected as they already have the dependency on sale-workflow.

@sebastienbeau
Copy link
Member

@bguillot Ithink also that it's will be a better idea to move sale_payment_method in sale_workflow

bguillot and others added 3 commits July 9, 2015 10:50
…sale-workflow into 7.0_add_sale_credit_note

Conflicts:
	sale_credit_note/invoice.py
@sebastienbeau
Copy link
Member

Hi after some month of using in production this module, and some customer crazy case, we work on a new version that will be quite different than the current one. So I close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants