Skip to content

[ADD] hr_payroll_document_queue#252

Open
SirPyTech wants to merge 2 commits into
OCA:16.0from
PyTech-SRL:16.0-add-hr_payroll_document_queue
Open

[ADD] hr_payroll_document_queue#252
SirPyTech wants to merge 2 commits into
OCA:16.0from
PyTech-SRL:16.0-add-hr_payroll_document_queue

Conversation

@SirPyTech
Copy link
Copy Markdown

Add a new module to process the payslips asynchronously.

I have also included a small improvement to allow multiple wizards to be processed at the same time without conflicts.
A more elaborate solution could be implemented (like using https://docs.python.org/3/library/tempfile.html) but that would require more refactoring.

Let me know what you think!

If multiple wizards are sending payrolls at the same time, every wizard acts in their own folder.
Also add display name.
@OCA-git-bot
Copy link
Copy Markdown
Contributor

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

@SirPyTech SirPyTech marked this pull request as ready for review February 11, 2026 11:15
Copy link
Copy Markdown

@quirino95 quirino95 left a comment

Choose a reason for hiding this comment

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

Code and functional review: LGTM

Suggestion (non-blocking): What do you think about keeping just the "Send" button instead of two? Of course, in this case, "Send" will do an asynchronous call. I think this is better for the user experience.

Copy link
Copy Markdown

@anusriNPS anusriNPS left a comment

Choose a reason for hiding this comment

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

Code and Functional Review: LGTM.

Observation: After emails sent, could not check notifications in Odoo chatter box about emails.

@OCA-git-bot
Copy link
Copy Markdown
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). 🤖

@SirPyTech
Copy link
Copy Markdown
Author

Thanks both for your reviews!

Suggestion (non-blocking): What do you think about keeping just the "Send" button instead of two? Of course, in this case, "Send" will do an asynchronous call. I think this is better for the user experience.

@quirino95 I prefer to show explicitly what's going to happen, one button or the other can be hidden for specific users adding user groups.


Observation: After emails sent, could not check notifications in Odoo chatter box about emails.

@anusriNPS we already talked about this and that was because jobs weren't starting, is this still happening?

@anusriNPS
Copy link
Copy Markdown

Observation: After emails sent, could not check notifications in Odoo chatter box about emails.

@anusriNPS we already talked about this and that was because jobs weren't starting, is this still happening?

I was able to verify email notification from mailhog but I thought it would be good to notify in Odoo chatter box about mail sent information for user of payroll management or provide some information about notification related to employee details and so on. Currently, only notification says Payroll sent successfully which is not intuitive. This observation is also related to hr_payroll_document_queue in general not specific to this PR.

@SirPyTech
Copy link
Copy Markdown
Author

Observation: After emails sent, could not check notifications in Odoo chatter box about emails.

@anusriNPS we already talked about this and that was because jobs weren't starting, is this still happening?

I was able to verify email notification from mailhog but I thought it would be good to notify in Odoo chatter box about mail sent information for user of payroll management or provide some information about notification related to employee details and so on. Currently, only notification says Payroll sent successfully which is not intuitive. This observation is also related to hr_payroll_document_queue in general not specific to this PR.

I think we are mixing up the 2 types of notifications that the user can select: image
If the user selects "Handle by Emails", they will only get an email; if they select "Handle in Odoo", they will only get a notification in Odoo.

If they want Odoo notifications, then in the chatter there will be a message with the same subject of the sent payrolls image
and in Discuss they'll be able to also open the wizard's record: image, and then image.

I agree that it could be improved but if you have a look at the code, I simply tell Odoo to notify the user with self.env["mail.thread"].message_notify and then everything follows the standard flow.

@anusriNPS
Copy link
Copy Markdown

Observation: After emails sent, could not check notifications in Odoo chatter box about emails.

@anusriNPS we already talked about this and that was because jobs weren't starting, is this still happening?

I was able to verify email notification from mailhog but I thought it would be good to notify in Odoo chatter box about mail sent information for user of payroll management or provide some information about notification related to employee details and so on. Currently, only notification says Payroll sent successfully which is not intuitive. This observation is also related to hr_payroll_document_queue in general not specific to this PR.

I think we are mixing up the 2 types of notifications that the user can select: image If the user selects "Handle by Emails", they will only get an email; if they select "Handle in Odoo", they will only get a notification in Odoo.

If they want Odoo notifications, then in the chatter there will be a message with the same subject of the sent payrolls image and in Discuss they'll be able to also open the wizard's record: image, and then image.

I agree that it could be improved but if you have a look at the code, I simply tell Odoo to notify the user with self.env["mail.thread"].message_notify and then everything follows the standard flow.

Thank you for the detailed response. It helped to understand different notifications sent as per configuration. Agree, we are following standard flow here. Discussion was helpful to understand the module better.

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.

4 participants