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

[17.0][FIX] account_reconcile_oca: use the right parameters on views #653

Merged
merged 1 commit into from
May 24, 2024

Conversation

xtanuiha
Copy link

@xtanuiha xtanuiha commented Apr 29, 2024

Steps to Reproduce the Error.

  1. Click menu Accounting\Configuration\Accounting\Chart of Accounts
  2. Click Reconcile button in list view
  3. Raise error below
UncaughtPromiseError > EvalError
Uncaught Promise > Can not evaluate python expression: ([("account_id", "=", id)]) Error: Name 'id' is not defined

image

@OCA-git-bot
Copy link
Contributor

Hi @etobella,
some modules you are maintaining are being modified, check this out!

@xtanuiha xtanuiha marked this pull request as ready for review April 29, 2024 08:35
@etobella
Copy link
Member

When is the error raised? Can you give a step by step explanation please?

@xtanuiha xtanuiha changed the title [17.0][Fix] Fix error when click the reconcie button in the account.account list [17.0][FIX] Fix error when click the reconcie button in the account.account list Apr 29, 2024
@xtanuiha
Copy link
Author

When is the error raised? Can you give a step by step explanation please?

Thank you for the feedback. The Pull Request description has been updated to include steps to reproduce the error.

@etobella
Copy link
Member

etobella commented May 9, 2024

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 17.0.

@etobella
Copy link
Member

etobella commented May 9, 2024

Thanks! I see the error. The right way to solve it, would be to edit here
https://github.com/OCA/account-reconcile/blob/17.0/account_reconcile_oca/views/account_account_reconcile.xml#L153

and use the domain [('account_id', '=', active_id)]

@xtanuiha
Copy link
Author

Thanks! I see the error. The right way to solve it, would be to edit here https://github.com/OCA/account-reconcile/blob/17.0/account_reconcile_oca/views/account_account_reconcile.xml#L153

and use the domain [('account_id', '=', active_id)]

Thank you for your suggestion, I have corrected the code, please review it.

@etobella
Copy link
Member

Great! It looks good. Can you squash your commits in a single one and use a better commit message? For example [FIX] account_reconcile_oca: use the right parameters on views

@xtanuiha xtanuiha changed the title [17.0][FIX] Fix error when click the reconcie button in the account.account list [FIX] account_reconcile_oca: use the right parameters on views May 24, 2024
@xtanuiha xtanuiha changed the title [FIX] account_reconcile_oca: use the right parameters on views [17.0][FIX] account_reconcile_oca: use the right parameters on views May 24, 2024
@xtanuiha
Copy link
Author

Great! It looks good. Can you squash your commits in a single one and use a better commit message? For example [FIX] account_reconcile_oca: use the right parameters on views

Squash complete, thanks!

@pedrobaeza
Copy link
Member

/ocabot rebase

@etobella
Copy link
Member

One question. why the history is weird? I mean, you are commiting as OCA-git-bot?

@pedrobaeza
Copy link
Member

He has rebased over an old branch. That's why I launched a rebase from the bot, but it's not working. @sbidoul is the bot stalled?

@OCA-git-bot
Copy link
Contributor

@pedrobaeza The rebase process failed, because command git rebase origin/17.0 failed with output:

First, rewinding head to replay your work on top of it...
Applying: Added translation using Weblate (Italian)
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
CONFLICT (add/add): Merge conflict in account_reconcile_oca/i18n/it.po
Auto-merging account_reconcile_oca/i18n/it.po
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 Added translation using Weblate (Italian)
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

@pedrobaeza
Copy link
Member

The rebase + conflict resolution must be done by @xtanuiha

@xtanuiha
Copy link
Author

@etobella @pedrobaeza
Ready for merge, thanks!

@pedrobaeza
Copy link
Member

I'm afraid not. Now there are 9 commits. You are still doing rebases on old branches. You should fetch first the last content here on OCA. If your OCA remote is origin, and being on the branch of this PR:

git fetch origin 17.0
git rebase origin/17.0
git push -f

@xtanuiha
Copy link
Author

I'm afraid not. Now there are 9 commits. You are still doing rebases on old branches. You should fetch first the last content here on OCA. If your OCA remote is origin, and being on the branch of this PR:

git fetch origin 17.0
git rebase origin/17.0
git push -f

Thank you very much for your help! Could you please take a look and confirm if the current status of this PR is correct?

@pedrobaeza pedrobaeza added this to the 17.0 milestone May 24, 2024
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.

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-653-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 082606d into OCA:17.0 May 24, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 94b9223. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants