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

[14.0][FIX] account_statement_import_coda: note should only contain info of the current move #140

Merged
merged 1 commit into from
Jan 30, 2024
Merged

[14.0][FIX] account_statement_import_coda: note should only contain info of the current move #140

merged 1 commit into from
Jan 30, 2024

Conversation

marielejeune
Copy link
Contributor

@marielejeune marielejeune commented May 19, 2022 •
edited
Loading

Information about all transaction lines (name, bank account, address) were repeated in note field od each transaction line
wrong_transaction

With this modification we want in a transaction line to stock only the information of the current line.

good_transaction

@marielejeune marielejeune changed the title [FIX] note only contains info of the current move [14.0][FIX] account_statement_import_coda: note only contains info of the current move May 19, 2022
@marielejeune marielejeune mentioned this pull request Nov 17, 2022
Merged
@sbidoul
Copy link
Member

sbidoul commented Nov 17, 2022

@marielejeune could you update the commit message with some explanation about why the change is necessary? It's not obvious to me by just looking at the diff and PR title.

@marielejeune marielejeune force-pushed the 14.0-mig-account_bank_statement_import_coda branch from 32783a3 to 0fb39bb Compare November 17, 2022 16:38
@marielejeune
Copy link
Contributor Author

@marielejeune could you update the commit message with some explanation about why the change is necessary? It's not obvious to me by just looking at the diff and PR title.

Done

@marielejeune marielejeune changed the title [14.0][FIX] account_statement_import_coda: note only contains info of the current move [14.0][FIX] account_statement_import_coda: note should only contain info of the current move Jan 19, 2023
@marielejeune marielejeune force-pushed the 14.0-mig-account_bank_statement_import_coda branch from 0fb39bb to d2eb9da Compare January 19, 2023 15:40
@marielejeune marielejeune force-pushed the 14.0-mig-account_bank_statement_import_coda branch from d2eb9da to 621a8a9 Compare January 19, 2023 15:56
@github-actions
Copy link

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 Oct 29, 2023
robinkeunen
robinkeunen approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@robinkeunen robinkeunen 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 patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-140-by-robinkeunen-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 7, 2023
@OCA-git-bot
Merge PR #140 into 14.0
2533f57 
Signed-off-by robinkeunen
@OCA-git-bot
Copy link
Contributor

@robinkeunen your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-140-by-robinkeunen-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.

@robinkeunen
Copy link
Contributor

@marielejeune indeed, precommit failed after rebasing on 14.0. Can you have a look at this #206 before rebasing ?

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 12, 2023
@marielejeune
[FIX] note only contains info of the current move
6ce7db5 
The communication given in a line of a coda file contains a lot of information (such as
the partner's name and home address, the bank's name and identification, the structured
communication...). This information is what we get in transaction_ref.
But in the move communication we only want to keep the actual payment transaction
communication, which is given in ref_move
@marielejeune marielejeune force-pushed the 14.0-mig-account_bank_statement_import_coda branch from 621a8a9 to 6ce7db5 Compare November 16, 2023 09:17
@marielejeune
Copy link
Contributor Author

@robinkeunen rebased on 14.0 and pipeline is green

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@robinkeunen
Copy link
Contributor

/ocabot merge patch

@marielejeune
Copy link
Contributor Author

Hi @robinkeunen can you see why your merge command didn't finish? Thanks

@sbidoul
Copy link
Member

sbidoul commented Jan 30, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-140-by-sbidoul-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 2192d7f into OCA:14.0 Jan 30, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 685f297. Thanks a lot for contributing to OCA. ❤️

@sbidoul sbidoul deleted the 14.0-mig-account_bank_statement_import_coda branch January 30, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbidoul sbidoul sbidoul left review comments

@acsonefho acsonefho acsonefho approved these changes

@robinkeunen robinkeunen robinkeunen approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marielejeune @sbidoul @OCA-git-bot @robinkeunen @acsonefho