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][ADD] hr_timesheet_name_customer #568

Conversation

solo4games
Copy link

This module added new field Customer Description in list view In this module in report pdf instead of Description you can see Customer Description. This field can see only Timesheets:Administrator

@solo4games solo4games marked this pull request as draft February 15, 2023 13:24
"""override create method, initialize name_customer"""

@api.model
def create(self, vals_list):
Copy link
Member

Choose a reason for hiding this comment

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

Starting Odoo 12 you can create several record and once using create_multi decorator.
Check example here


@api.model
def create(self, vals_list):
if vals_list["name_customer"] is False:
Copy link
Member

Choose a reason for hiding this comment

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

Just do
If not vals.get(“name_customer”):

Because if name_customer is not present in Val’s vals[“name_customer”] will trigger an exception

{
"name": "Name Customer",
"summary": "Add Description Customer",
"version": "16.0.0.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Pleas follow version numbering rules. You can check them here


{
"name": "Name Customer",
"summary": "Add Description Customer",
Copy link
Member

Choose a reason for hiding this comment

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

Make description more clear for end user. Eg
“Add ‘Description Customer’ field for timesheets”

"views/hr_timesheet_name_customer_views.xml",
"report/name_customer_template.xml",
],
"demo": [],
Copy link
Member

Choose a reason for hiding this comment

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

Do not add empty keys. Remove it please

@solo4games solo4games force-pushed the 16.0-t2358-oca_timesheet-add_customer_description branch from e6b5724 to 3c7b447 Compare February 15, 2023 15:34
{
"name": "Name Customer",
"summary": "Add ‘Description Customer’ field for timesheets",
"version": "16.0.1.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

This is still a release version so 16.0.1.0.0

@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8" ?>
<odoo>
<record id="hr_timesheet_name_customer_view" model="ir.ui.view">
Copy link
Member

Choose a reason for hiding this comment

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

Please follow guidelines when for inherited xml ids

# customer partner
cls.partner = cls.env["res.partner"].create(
{
"name": "Customer Task",
Copy link
Member

Choose a reason for hiding this comment

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

Not the best name for Customer)
It might be misleading

from odoo.tests.common import TransactionCase


class TestCommonNameCustomer(TransactionCase):

Choose a reason for hiding this comment

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

I think this class you can move to new common.py file and them make import to current file.



class TestTimesheet(TestCommonNameCustomer):
def setUp(self):

Choose a reason for hiding this comment

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

You can delete this.

<field name="model">account.analytic.line</field>
<field name="inherit_id" ref="hr_timesheet.hr_timesheet_line_tree" />
<field name="arch" type="xml">
<xpath expr="//field[@name='name']" position="after">

Choose a reason for hiding this comment

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

You can replace on <field name="name" position="after">...</field>

Copy link

@geomer198 geomer198 left a comment

Choose a reason for hiding this comment

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

LGTM
PR Name must be have format [verion][ADD] module_name

# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl-3.0).

{
"name": "Name Customer",
Copy link
Member

Choose a reason for hiding this comment

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

Timesheet Description Customer

"application": False,
"installable": True,
"depends": ["hr_timesheet"],
"data": [
Copy link
Member

Choose a reason for hiding this comment

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

Add "maintainers" key with:

  • CetmixGitDrone
  • You

See example here

@@ -0,0 +1,5 @@
To use this module you need to:

#. Go to Apps and install this module
Copy link
Member

Choose a reason for hiding this comment

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

Modify "Description Customer" field value. Select timesheets you want to print add choose "Timesheets Customer" report.
"Description" column values will be populated from the "Description Customer" field.

Copy link
Member

@ivs-cetmix ivs-cetmix left a comment

Choose a reason for hiding this comment

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

LGTM!
please move squash commits and move out of "draft"

@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). 🤖

@solo4games solo4games force-pushed the 16.0-t2358-oca_timesheet-add_customer_description branch 3 times, most recently from f617766 to 3786e67 Compare February 26, 2023 20:16
@solo4games solo4games changed the title [16.0][ADD] hr_timesheet_name_customer:custom desc [16.0][ADD] hr_timesheet_name_customer Feb 26, 2023
@solo4games solo4games marked this pull request as ready for review February 26, 2023 20:21
@ivs-cetmix
Copy link
Member

Hey @OCA/human-resources-maintainers could you please have a look at this?

Copy link
Member

@Saran440 Saran440 left a comment

Choose a reason for hiding this comment

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

@@ -0,0 +1 @@

Copy link
Member

Choose a reason for hiding this comment

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

please delete this file or generate readme from https://github.com/OCA/maintainer-tools#readme-generator

Copy link
Member

Choose a reason for hiding this comment

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

please delete this file or generate readme from https://github.com/OCA/maintainer-tools#readme-generator

If you delete it then pre-commit will fail. That’s why we are simply adding a file with some placeholder in it. It will be overwritten anyway

Copy link
Member

Choose a reason for hiding this comment

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

please delete this file or generate readme from https://github.com/OCA/maintainer-tools#readme-generator

@Saran440 could you please review its once again? Would be great to have it merged

@Saran440
Copy link
Member

Code Reviewed. 👍

please rebase commit into 1 commit.

@solo4games solo4games force-pushed the 16.0-t2358-oca_timesheet-add_customer_description branch 2 times, most recently from 726189c to 10ddf55 Compare June 28, 2023 18:59
This module added new field Customer Description in list view
In this module in report pdf instead of Description you can see Customer
Description. This field can see only Timesheets:Administrator
@solo4games solo4games force-pushed the 16.0-t2358-oca_timesheet-add_customer_description branch from 10ddf55 to 53ad149 Compare June 28, 2023 19:09
@ivs-cetmix
Copy link
Member

Code Reviewed. 👍

please rebase commit into 1 commit.

done

@Saran440
Copy link
Member

Saran440 commented Jul 1, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-568-by-Saran440-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b922c73 into OCA:16.0 Jul 1, 2023
5 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

None yet

5 participants