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

[16.0][IMP] account_payment_order: check that bank allows out-payments #1177

Closed
wants to merge 2 commits into from
Closed

[16.0][IMP] account_payment_order: check that bank allows out-payments #1177

wants to merge 2 commits into from

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Nov 14, 2023 •
edited
Loading

Add check that bank allows out-payments

@astirpe astirpe force-pushed the 16_imp_account_payment_order branch 2 times, most recently from 75bebca to ddfd5d9 Compare November 14, 2023 13:12
@astirpe astirpe changed the title WIP [16.0][IMP] account_payment_order: check that bank allows out payments WIP [16.0][IMP] account_payment_order: check that bank allows out-payments Nov 14, 2023
astirpe added a commit to onesteinbv/addons-oca that referenced this pull request Nov 21, 2023
@astirpe
[IMP] account_payment_order: check that bank allows out-payments (OCA…
d22a5a7 
…/bank-payment#1177)
@astirpe astirpe changed the title WIP [16.0][IMP] account_payment_order: check that bank allows out-payments [16.0][IMP] account_payment_order: check that bank allows out-payments Nov 21, 2023
@astirpe astirpe marked this pull request as ready for review November 21, 2023 11:28
flotho
flotho approved these changes Jan 29, 2024
View reviewed changes
Copy link
Member

@flotho flotho 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, thanks LGTM

@flotho
Copy link
Member

flotho commented Jan 29, 2024

Hi @astirpe cuold you re-trigger runboat with a commit ?

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 2, 2024

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 2, 2024
@pedrobaeza pedrobaeza added this to the 16.0 milestone Jun 3, 2024
pedrobaeza
pedrobaeza approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

.

pedrobaeza
pedrobaeza requested changes Jun 3, 2024
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Wait, I'm seeing that this is not enough, as we should filter these accounts in:

  • Automatic selection when no bank is selected in the invoice.
  • When displaying the account in the invoice if the payment mode has this configuration.

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1177-by-pedrobaeza-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jun 3, 2024
@OCA-git-bot
Merge PR #1177 into 16.0
523b946 
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1177-by-pedrobaeza-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 9, 2024
@sbidoul
Copy link
Member

sbidoul commented Aug 22, 2024

I only now notice this implements #1330

To me this PR looks good. I think it's fine to let users prepare payment orders, and block when generating.

@astirpe could you rebase ?

pedrobaeza
pedrobaeza approved these changes Aug 22, 2024
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK, let's go with this option

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1177-by-pedrobaeza-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Aug 22, 2024
@OCA-git-bot
Merge PR #1177 into 16.0
08f8f89 
Signed-off-by pedrobaeza
@pedrobaeza pedrobaeza mentioned this pull request Aug 22, 2024
Closed
@pedrobaeza
Copy link
Member

Please fw-port it to 17.0

@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1177-by-pedrobaeza-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@pedrobaeza
Copy link
Member

pedrobaeza commented Aug 22, 2024 •
edited
Loading

Uhm, it seems there are affected tests by this change. Can you please rebase and check them?

lmarion-source added a commit to acsone/bank-payment that referenced this pull request Aug 30, 2024
@lmarion-source
[16.0][IMP] Take allow_out_payment into account on payment order
5f1064e 
    A previous MR existed but only when payment order was triggered from invoices.
    We added the case also directement from confirm button on payment order

original PR: OCA#1177
@lmarion-source lmarion-source mentioned this pull request Aug 30, 2024
Open
lmarion-source added a commit to acsone/bank-payment that referenced this pull request Sep 11, 2024
@lmarion-source
[16.0][IMP] Take allow_out_payment into account on payment order
a02bcff 
        A previous MR existed but only when payment order was triggered from invoices.
        We added the case also directement from confirm button on payment order

    original PR: OCA#1177
@pedrobaeza
Copy link
Member

Superseded by #1340

@pedrobaeza pedrobaeza closed this Sep 12, 2024
lmarion-source added a commit to acsone/bank-payment that referenced this pull request Sep 13, 2024
@lmarion-source
[16.0][IMP] Take allow_out_payment into account on payment order
565a13b 
    A previous MR existed but only when payment order was triggered from invoices.
    We added the case also directement from confirm button on payment order

    original PR: OCA#1177
lmarion-source added a commit to acsone/bank-payment that referenced this pull request Sep 26, 2024
@lmarion-source
[16.0][IMP] Take allow_out_payment into account on payment order
5be3c4b 
   A previous MR existed but only when payment order was triggered from invoices.
   We added the case also directement from confirm button on payment order

   original PR: OCA#1177
lmarion-source added a commit to acsone/bank-payment that referenced this pull request Sep 26, 2024
@lmarion-source
[16.0][IMP] Take allow_out_payment into account on payment order
1cd2603 
    A previous MR existed but only when payment order was triggered from invoices.
    We added the case also directement from confirm button on payment order

    original PR: OCA#1177
lmarion-source added a commit to acsone/bank-payment that referenced this pull request Sep 26, 2024
@lmarion-source
[16.0][IMP] Take allow_out_payment into account on payment order
adb3c59 
    A previous MR existed but only when payment order was triggered from invoices.
    We added the case also directement from confirm button on payment order

    original PR: OCA#1177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexis-via alexis-via alexis-via left review comments

@pedrobaeza pedrobaeza pedrobaeza approved these changes

@flotho flotho flotho approved these changes

Assignees
No one assigned
Labels
approved
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
6 participants
@astirpe @flotho @OCA-git-bot @sbidoul @pedrobaeza @alexis-via