Skip to content

[ADD] Payroll: adding new reporting doc #4219

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
Closed
wants to merge 1 commit into from
Closed

[ADD] Payroll: adding new reporting doc #4219

wants to merge 1 commit into from

Conversation

larm-odoo
Copy link
Contributor

New doc for the reporting section of Payroll

@robodoo
Copy link
Collaborator

robodoo commented Apr 20, 2023

Pull request status dashboard.

@C3POdoo C3POdoo requested review from a team April 20, 2023 15:53
@samueljlieber samueljlieber removed the request for review from a team April 20, 2023 20:44
@samueljlieber samueljlieber force-pushed the 14.0-payroll-new-reporting-doc-larm branch from e3fb4b5 to c515512 Compare April 20, 2023 20:56
@samueljlieber
Copy link
Contributor

Fixed build error in c515512 due to the h1 missing the top level of ====.

@larm-odoo before making edits to this PR locally, please pull down this change using git pull --rebase 🙂

@samueljlieber samueljlieber requested review from a team April 20, 2023 20:59
jcs-odoo
jcs-odoo requested changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@jcs-odoo jcs-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

This review is only about payroll.rst, for the review of the structure.

hojo-odoo
hojo-odoo suggested changes Apr 24, 2023
View reviewed changes
@larm-odoo larm-odoo force-pushed the 14.0-payroll-new-reporting-doc-larm branch from c515512 to 065c6aa Compare April 25, 2023 16:04
@larm-odoo
Copy link
Contributor Author

Thanks for the review @hojo-odoo! I incorporated all the changes you suggested.

@larm-odoo larm-odoo force-pushed the 14.0-payroll-new-reporting-doc-larm branch from 065c6aa to 5670ee0 Compare April 26, 2023 15:52
@larm-odoo larm-odoo requested a review from hojo-odoo April 26, 2023 15:53
@larm-odoo larm-odoo requested a review from jcs-odoo May 9, 2023 16:39
hojo-odoo
hojo-odoo suggested changes May 9, 2023
View reviewed changes
Copy link
Contributor

@hojo-odoo hojo-odoo 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.

Hey @larm-odoo,

Just a few minor updates left to make. Also, there should be an empty line at the end of the doc.

Thanks,
hojo

jcs-odoo
jcs-odoo reviewed May 9, 2023
View reviewed changes
Copy link
Contributor

@jcs-odoo jcs-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! Similarly to my comment in the pr about Recruitment, please use the tag [ADD] instead of NEW in your commit message and PR title.

Thanks :)

@larm-odoo larm-odoo changed the title [NEW] Payroll: adding new reporting doc [ADD] Payroll: adding new reporting doc May 11, 2023
@larm-odoo larm-odoo force-pushed the 14.0-payroll-new-reporting-doc-larm branch from 5670ee0 to e78bd2a Compare May 11, 2023 18:49
@larm-odoo larm-odoo requested review from hojo-odoo and jcs-odoo May 11, 2023 18:51
@larm-odoo
Copy link
Contributor Author

hi @jcs-odoo - I missed this one, I didn't go back through the older PR's and I'll make sure everything has the correct tags today! @hojo-odoo , I made the edits you suggested plus I found a coupe more instances that could use a GUI label, and a few "smart" buttons that weren't actually "smart" buttons, just regular, so I made those changes, too.

@jcs-odoo jcs-odoo removed their request for review June 9, 2023 16:51
@larm-odoo larm-odoo force-pushed the 14.0-payroll-new-reporting-doc-larm branch from e78bd2a to 177c101 Compare August 25, 2023 19:46
@StraubCreative
Copy link
Contributor

Hi @larm-odoo
Regarding the commit message name:
In addition to editing that here on GH you'll want to do that in the terminal too, as the tag still says [NEW]. To change your commit message in the terminal, use the git commit --amend command 👍

@larm-odoo larm-odoo force-pushed the 14.0-payroll-new-reporting-doc-larm branch from 177c101 to 0f16398 Compare August 28, 2023 14:29
@larm-odoo
Copy link
Contributor Author

Thanks @StraubCreative - I didn't realize there were a couple spots to change that! I will fix my other PR's where I made the same mistake, and only did half the solution =) This is all set.

@meng-odoo meng-odoo requested a review from a team December 14, 2023 22:11
meng-odoo
meng-odoo approved these changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@meng-odoo meng-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! This doc is looking great. I had a few suggestions, but I'm approving now. Please go ahead and tag Sam for technical review after you've gone through my comments. Thanks!

@larm-odoo larm-odoo force-pushed the 14.0-payroll-new-reporting-doc-larm branch from 0f16398 to e7a2ef4 Compare December 14, 2023 23:05
StraubCreative
StraubCreative requested changes Dec 18, 2023
View reviewed changes
Copy link
Contributor

@StraubCreative StraubCreative 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

Good doc. I just caught a few things which you can read below.

For necessary items, I'll fix quickly and push up in a following commit and merge the doc. For everything else, just some things to look at for next time 😉

Thank you for your diligent and thorough work on this!

content/applications/hr/payroll/reporting.rst
Comment on lines +50 to +54
options to display. The options available include: :guilabel:`# Payslip`, :guilabel:`Basic Wage`,
:guilabel:`Basic Wage for Time Off`, :guilabel:`Days of Paid Time Off`, :guilabel:`Days of
Unforeseen Absence`, :guilabel:`Days of Unpaid Time Off`, :guilabel:`Gross Wage`, :guilabel:`Net
Wage`, :guilabel:`Number of Days`, :guilabel:`Number of Hours`, :guilabel:`Work Days`,
:guilabel:`Work Hours`, and :guilabel:`Count`.
Copy link
Contributor

Choose a reason for hiding this comment

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

would list these out in a bullet list and define them

Copy link
Contributor

Choose a reason for hiding this comment

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

omitting, can do later.

content/applications/hr/payroll/reporting.rst
:align: center
:alt: Report dashboard view.

Line chart
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: for the three chart options here we can use content tabs

Copy link
Contributor

Choose a reason for hiding this comment

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

omitting, can do later.

@StraubCreative StraubCreative force-pushed the 14.0-payroll-new-reporting-doc-larm branch from e7a2ef4 to 2464aa9 Compare December 18, 2023 23:20
@StraubCreative
Copy link
Contributor

Necessary change requests for merge implemented in 2464aa9.

@StraubCreative StraubCreative force-pushed the 14.0-payroll-new-reporting-doc-larm branch from 2464aa9 to 3078445 Compare December 18, 2023 23:35
@StraubCreative
Copy link
Contributor

Revised metadata for payroll.rst in 3078445.

Plentiful body content and additional docs in this scope have been added since JCS's original change request to omit metadata, so the following have been added to the top of the file:

:show-content:
:show-toc:

StraubCreative
StraubCreative approved these changes Dec 18, 2023
View reviewed changes
Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

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

All checks pass ✅

@robodoo r+

robodoo pushed a commit that referenced this pull request Dec 18, 2023
@larm-odoo @StraubCreative
[ADD] Payroll: adding new reporting doc
e5e9a54 
closes #4219

Signed-off-by: Zachary Straub (zst) <zst@odoo.com>
@robodoo
Copy link
Collaborator

robodoo commented Dec 19, 2023

@larm-odoo @StraubCreative staging failed: ci/runbot on 9b2d47c6820f0b2d8d3391c1c3e42cb9894d2540 (view more at https://runbot.odoo.com/runbot/build/55315519)

@StraubCreative
Copy link
Contributor

@robodoo retry

robodoo pushed a commit that referenced this pull request Dec 19, 2023
@larm-odoo @StraubCreative
[ADD] Payroll: adding new reporting doc
a6274ff 
closes #4219

Signed-off-by: Zachary Straub (zst) <zst@odoo.com>
@fw-bot fw-bot mentioned this pull request Dec 19, 2023
Closed
@robodoo robodoo closed this Dec 19, 2023
This was referenced Dec 19, 2023
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcs-odoo jcs-odoo jcs-odoo requested changes

@StraubCreative StraubCreative StraubCreative approved these changes

@hojo-odoo hojo-odoo Awaiting requested review from hojo-odoo

@samueljlieber samueljlieber Awaiting requested review from samueljlieber

+1 more reviewer

@meng-odoo meng-odoo meng-odoo approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@larm-odoo @robodoo @samueljlieber @StraubCreative @jcs-odoo @meng-odoo @hojo-odoo