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

[16.0][MIG] pos_report_session_summary #1190

Closed
wants to merge 20 commits into from

Conversation

zamberjo
Copy link
Member

@zamberjo zamberjo commented May 6, 2024

No description provided.

@legalsylvain
Copy link
Contributor

/ocabot migration pos_report_session_summary

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jul 2, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Jul 2, 2024
37 tasks
@carlos-lopez-tecnativa
Copy link
Contributor

@zamberjo Could you update the status of this PR? I am interested in the migration.

@zamberjo zamberjo force-pushed the 16.0-mig-pos_report_session_summary branch from 28809dc to e8a341d Compare July 19, 2024 12:49
Copy link
Contributor

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

@zamberjo I tested and encountered an error. Please check my comments.

Additionally, you should split commits (one for precommit and another for migration) according to the guidelines in https://github.com/oca/maintainer-tools/wiki/Migration-to-version-16.0

TT49806

</tr>
</thead>
<tbody>
<tr t-foreach="o.statement_ids" t-as="statement">
Copy link
Contributor

Choose a reason for hiding this comment

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

This field not exist anymore
image

<tr t-foreach="o.statement_ids" t-as="statement">
<td><span t-field="statement.name" /></td>
<td><span t-field="statement.journal_id" /></td>
<td class="text-right">
Copy link
Contributor

Choose a reason for hiding this comment

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

Update class to BT5
https://getbootstrap.com/docs/5.0/migration/#utilities

Suggested change
<td class="text-right">
<td class="text-end">

@zamberjo zamberjo force-pushed the 16.0-mig-pos_report_session_summary branch from e8a341d to 19e7ad4 Compare July 24, 2024 07:02
@zamberjo
Copy link
Member Author

Hi @carlos-lopez-tecnativa , I just did a rebase, I'm going to take this up again and do what you told me.

I will let you know as soon as I have it ready for review.

@zamberjo zamberjo force-pushed the 16.0-mig-pos_report_session_summary branch 2 times, most recently from 1969181 to 1444855 Compare July 24, 2024 07:12
@zamberjo zamberjo force-pushed the 16.0-mig-pos_report_session_summary branch from 1444855 to b68b6a8 Compare July 24, 2024 07:17
Copy link
Sponsor

@celm1990 celm1990 left a comment

Choose a reason for hiding this comment

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

@zamberjo I left my comments. If you have any questions, please let me know.

</thead>
<tbody>
<tr
t-foreach="o.statement_line_ids.mapped('statement_id')"
Copy link
Sponsor

Choose a reason for hiding this comment

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

I think it is better to use pos.payment instead of account.bank.statement.line. Please review this model and field. If pos.session does not have a field to pos.payment, you can add this field as one2many or include it in the report by performing a read_group to get all related payment information.
Remember that account.bank.statement is no longer required and Odoo does not create records in this model from POS.
image

Comment on lines +106 to +107
<th>Partner</th>
<th>Partner</th>
Copy link
Sponsor

Choose a reason for hiding this comment

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

Because this is repeat twice?

@carlos-lopez-tecnativa
Copy link
Contributor

@zamberjo, could you please update the status of this PR?

@zamberjo
Copy link
Member Author

zamberjo commented Aug 1, 2024

Hi @carlos-lopez-tecnativa , unfortunately I have been caught in the middle of several migrations and holydays. If you need it very urgently you can continue it since I don't know when I will be able to resume it.

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