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

[ADD] Payroll: Salary attachment report #11064

Closed
wants to merge 1 commit into from

Conversation

larm-odoo
Copy link
Contributor

Adding the salary attachment reporting doc as a separate stand-alone doc instead of combined in a reporting doc with all reports on it.

Original task card for this PR.

@robodoo
Copy link
Collaborator

robodoo commented Sep 19, 2024

Pull request status dashboard

@larm-odoo larm-odoo marked this pull request as draft September 19, 2024 21:35
@larm-odoo larm-odoo added the 5 label Sep 19, 2024
@larm-odoo larm-odoo force-pushed the 17.0-payroll-salary-attachment-report-larm branch from bf4a22f to f72316d Compare September 25, 2024 15:54
@larm-odoo larm-odoo force-pushed the 17.0-payroll-salary-attachment-report-larm branch from f72316d to 0bf202e Compare October 24, 2024 18:05
@larm-odoo larm-odoo marked this pull request as ready for review October 24, 2024 18:05
@C3POdoo C3POdoo requested a review from a team October 24, 2024 18:07
@larm-odoo larm-odoo force-pushed the 17.0-payroll-salary-attachment-report-larm branch from 0bf202e to cf6f875 Compare October 24, 2024 18:37
@larm-odoo
Copy link
Contributor Author

Hi @dikd-odoo - this is ready for a first-round review!

Copy link
Contributor

@dikd-odoo dikd-odoo left a comment

Choose a reason for hiding this comment

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

Hi @larm-odoo great work on this documentation! I did run into an issue where I was not able to find the Salary Attachment report under Payroll > Reporting. Might be something I missed, but could you double check if there is a configuration needed?

@larm-odoo larm-odoo force-pushed the 17.0-payroll-salary-attachment-report-larm branch from cf6f875 to 43dbc18 Compare October 28, 2024 21:09
@larm-odoo larm-odoo requested a review from a team October 28, 2024 21:10
@larm-odoo
Copy link
Contributor Author

Hi @dikd-odoo - thank you for your review! I put in a comment from one of your edits, that the report is a default one. If you go to a 17.0 runbot, there should be info in there, and the report is visible. LMK if you have any issues finding it!

@dikd-odoo
Copy link
Contributor

Hi @larm-odoo thank you for the clarification! I checked the flow in a 17.0 runbot and it went smoothly. This should be ready to go, with the note that the 18.0 will be updated to reflect configuration changes (Salary Attachment Report not a pre-configured report).

Let me know if you have any questions, thank you!

Copy link
Contributor

@ksc-odoo ksc-odoo left a comment

Choose a reason for hiding this comment

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

hey @larm-odoo -- just finished my Final Review. Only have a few notes that require your attention. Once you make those necessary adjustments, feel free to move this to Tech Review. Thanks!

content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@dikd-odoo dikd-odoo left a comment

Choose a reason for hiding this comment

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

Approving!

@larm-odoo larm-odoo force-pushed the 17.0-payroll-salary-attachment-report-larm branch from 43dbc18 to ca22ce1 Compare October 29, 2024 18:25
@larm-odoo larm-odoo requested a review from a team October 29, 2024 18:25
@larm-odoo
Copy link
Contributor Author

Hi @samueljlieber - this is ready for a tech review when you have a chance.

Copy link
Contributor

@samueljlieber samueljlieber left a comment

Choose a reason for hiding this comment

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

Hi @larm-odoo! Nice work- your changes look good to me, just a couple quick fixes.
.....
@robodoo delegate=larm-odoo

content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
content/applications/hr/payroll/salary_attachment.rst Outdated Show resolved Hide resolved
@larm-odoo larm-odoo force-pushed the 17.0-payroll-salary-attachment-report-larm branch from ca22ce1 to 99aec52 Compare October 31, 2024 16:50
@larm-odoo
Copy link
Contributor Author

@robodoo r+

robodoo pushed a commit that referenced this pull request Oct 31, 2024
closes #11064

Signed-off-by: Lara Martini (larm) <[email protected]>
robodoo pushed a commit that referenced this pull request Oct 31, 2024
closes #11064

Signed-off-by: Lara Martini (larm) <[email protected]>
@robodoo robodoo closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants