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

l10n_ch: Move ISR subscription fields under res.partner.bank #35873

Closed

Conversation

yvaucher
Copy link
Contributor

Description of the issue/feature this PR addresses:

An Inpayment Slip with Reference (ISR) subscription number is provided by the bank or by Postfinance
ISR When the ISR subscription comes from Postfinance it is a direct
subscription.
ISR (bank) When the ISR subscription comes from a bank it is the bank subscription
toward Postfinance.

Having the subscription number on res.bank has 2 drawbacks:

If you have multiple accounts at Postfinance
or
If you have multicompany and each company has a Postfinance account

you won't be able to emit with different ISR subscriber numbers.

This also renames the fields to be accurate on the meaning.

An ISR reference is a Payment reference that will apear on the invoice.
It is better to name it subscription number.

Current behavior before PR:

Fields for ISR subscription are misplaced. As it doesn't take into account the possibility of having an account at Postfinance and having a unique ISR subscription number.

Desired behavior after PR is merged:

Correct label and model for ISR subscription fields.

This PR only targets the model change. I'll address other issues in separated PRs.

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@yvaucher yvaucher force-pushed the l10n_ch_isr_subscription_fields_rename branch from dbda34c to da175f5 Compare August 20, 2019 17:06
@@ -16,7 +16,7 @@
</div>

<img class="swissqr" t-att-src="o.invoice_partner_bank_id.build_swiss_code_url(o.amount_residual, o.currency_id.name, o.invoice_date_due, o.partner_id, 'QRR',
(o.invoice_partner_bank_id.bank_id.l10n_ch_postal_eur) if (o.invoice_partner_bank_id.currency_id.name == 'EUR') else o.invoice_partner_bank_id.bank_id.l10n_ch_postal_chf,
(o.invoice_partner_bank_id.l10n_ch_isr_subscription_chf) if (o.invoice_partner_bank_id.currency_id.name == 'CHF') else o.invoice_partner_bank_id.l10n_ch_isr_subscription_chf,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I inverted the logic to have the default with CHF, as it is really more common to have CHF here.

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Aug 20, 2019
@yvaucher
Copy link
Contributor Author

@jco-odoo @hpr-odoo

@yvaucher yvaucher changed the title Move ISR subscription fields under res.partner.bank l10n_ch: Move ISR subscription fields under res.partner.bank Aug 21, 2019
@@ -15,8 +15,8 @@
class AccountMove(models.Model):
_inherit = 'account.move'

l10n_ch_isr_postal = fields.Char(compute='_compute_l10n_ch_isr_postal', help='The postal reference identifying the bank managing this ISR.')
l10n_ch_isr_postal_formatted = fields.Char(compute='_compute_l10n_ch_isr_postal', help="Postal reference of the bank, formated with '-' and without the padding zeros, to generate ISR report.")
l10n_ch_isr_subscription = fields.Char(compute='_compute_l10n_ch_isr_subscription', help='ISR subscribtion number identifying your company or your bank to generate ISR.')
Copy link
Contributor

Choose a reason for hiding this comment

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

subscribtion -> subscription

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch, fixed

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Aug 21, 2019
@jco-odoo
Copy link
Contributor

@yvaucher If you could just do the autosquash then we can merge from here (if it is green). I did the basic tests and it works.

But we need to check in the views (later). Maybe we need to add 2 related non-stored on the account journal like it is done with the postal code, because I suppose it is there where you will configure this.

@yvaucher
Copy link
Contributor Author

@jco-odoo Indeed, we will probably add those config either from the "Add a bank account" action.
Or after creation through the journal.

I rebase and squash now

An Inpayment Slip with Reference (ISR) subscription number is provided by the bank or by Postfinance
ISR When the ISR subscription comes from Postfinance it is a direct
subscription.
ISR (bank) When the ISR subscription comes from a bank it is the bank subscription
toward Postfinance.

Having the subscription number on res.bank has 2 drawbacks:

If you have multiple accounts at Postfinance
or
If you have multicompany and each company has a Postfinance account

you won't be able to emit with different ISR subscriber numbers.

This also renames the fields to be accurate on the meaning.
An ISR reference is a Payment reference that will apear on the invoice.
It is better to name it subscription number.
@yvaucher yvaucher force-pushed the l10n_ch_isr_subscription_fields_rename branch from 93f86ab to 2cd39e1 Compare August 21, 2019 12:51
@yvaucher
Copy link
Contributor Author

Last build was there http://runbot.odoo.com/runbot/build/595787

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Aug 21, 2019
@jco-odoo
Copy link
Contributor

@robodoo r+

@yvaucher yvaucher mentioned this pull request Aug 21, 2019
2 tasks
robodoo pushed a commit that referenced this pull request Aug 21, 2019
An Inpayment Slip with Reference (ISR) subscription number is provided by the bank or by Postfinance
ISR When the ISR subscription comes from Postfinance it is a direct
subscription.
ISR (bank) When the ISR subscription comes from a bank it is the bank subscription
toward Postfinance.

Having the subscription number on res.bank has 2 drawbacks:

If you have multiple accounts at Postfinance
or
If you have multicompany and each company has a Postfinance account

you won't be able to emit with different ISR subscriber numbers.

This also renames the fields to be accurate on the meaning.
An ISR reference is a Payment reference that will apear on the invoice.
It is better to name it subscription number.

closes #35873

Signed-off-by: Josse Colpaert <jco@openerp.com>
@robodoo
Copy link
Contributor

robodoo commented Aug 21, 2019

Merged at f3c1e6c, thanks!

@robodoo robodoo closed this Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants