[4548][IMP] l10n_jp_hr_employee, hr_employee_nrq, hr_employee_qualifications: change char to many2one for qualification name field #268

AungKoKoLin1997 · 2024-05-13T08:19:35Z

kanda999 · 2024-05-13T08:57:39Z

l10n_jp_hr_employee/views/qualification_name_views.xml

+        <field name="model">qualification.name</field>
+        <field name="arch" type="xml">
+            <tree editable="top">
+                <field name="name" />


Could you please add the sequence handle?

yostashiro · 2024-05-13T13:44:54Z

l10n_jp_hr_employee/models/qualification_name.py

+
+
+class QualificationName(models.Model):
+    _name = "qualification.name"


I find it a bit strange as a model name. How about hr.qualification or just qualification?

name field is exist in hr.qualification model and this model will have different type of qualifications. So I decided to use hr.qualification.type.

yostashiro

@kanda999 Please check the validity of my suggestions.

yostashiro · 2024-05-17T06:14:57Z

l10n_jp_hr_employee/models/hr_qualification.py

+    is_allowed_score = fields.Boolean(related="name.is_allowed_score", store=True)
+    score = fields.Char()


score makes the scope of the field too narrow. Let's allow more flexibility.

Suggested change

is_allowed_score = fields.Boolean(related="name.is_allowed_score", store=True)

score = fields.Char()

qualification_type_id = fields.Many2one("hr.qualification.type", required=True, string="Qualification Type")

needs_description = fields.Boolean(related="qualification_type_id.needs_description")

description = fields.Char()

No need to store needs_description.

yostashiro · 2024-05-17T06:16:09Z

l10n_jp_hr_employee/models/hr_qualification.py

@@ -15,14 +15,36 @@ class HrQualification(models.Model):
    _name = "hr.qualification"
    _order = "date_obtained"

-    name = fields.Char(required=True, help="e.g. CISA",)
+    name = fields.Many2one("hr.qualification.type", required=True)


Suggested change

name = fields.Many2one("hr.qualification.type", required=True)

name = fields.Char(compute="_compute_name", store=True)

We can get rid of the extensions of name_get() and name_search() this way.

yostashiro · 2024-05-17T10:37:06Z

l10n_jp_hr_employee/views/hr_qualification_type_views.xml

+        parent="hr.menu_hr_root"
+        sequence="4"
+        groups="base.group_user"


Suggested change

parent="hr.menu_hr_root"

sequence="4"

groups="base.group_user"

parent="hr.menu_human_resources_configuration"

sequence="10"

yostashiro · 2024-05-17T10:40:04Z

l10n_jp_hr_employee/models/hr_qualification_type.py

+
+    name = fields.Char(required=True, help="e.g. CISA")
+    sequence = fields.Integer(default=10)
+    needs_description = fields.Boolean(help="If enabled, allows input in the description field of the qualification.")


Suggested change

needs_description = fields.Boolean(help="If enabled, allows input in the description field of the qualification.")

needs_description = fields.Boolean(help="If enabled, description field of the qualification becomes required.")

yostashiro · 2024-05-17T14:08:53Z

l10n_jp_hr_employee/views/hr_private_info_views.xml

@@ -12,6 +12,7 @@
        <field name="res_model">hr.private.info</field>
        <field name="view_type">form</field>
        <field name="view_mode">tree,form</field>
+        <field name="context">{'readonly_by_pass': True}</field>


If we use this, we should add web_readonly_bypass as a dependency in the manifest file.

The question is, is it really worth adding the new module just for _onchange_qualification_name() (we should change the method name according to the field name, by the way, if we are to keep this method)?

Another approach is to just extend create() and write() and clear description when qualification_type_id is in vals and qualification_type_id.needs_description is false, instead of using the onchange method. I personally think this approach would be acceptable (the description value doesn't need to be cleared on the fly when qualification type is changed).

I think your idea is better because it can reduce the maintenance cost and simple.

Using create(), write() and onchange funcs still have some unexpected behavior. So, I decided to use onchange with the help of web_readonly_bypass.

@AungKoKoLin1997 OK, thanks for the persistence!

yostashiro

Code review.

kanda999

Code and Functional review.

yostashiro · 2024-06-05T13:50:28Z

l10n_jp_hr_employee/models/hr_private_info.py

+    recom_qualification_ids = fields.One2many(
        "hr.qualification",
        "private_info_id",
-        string="Qualification",
+        string="Recommended Qualification",
        context={"active_test": False},
+        domain=[('qualification_category','=','recommended')]
    )
+    non_recom_qualification_ids = fields.One2many(
+        "hr.qualification",
+        "private_info_id",
+        string="Non Recommended Qualification",
+        context={"active_test": False},
+        domain=[('qualification_category','=','non_recommended')]
+    )
+    lang_qualification_ids = fields.One2many(
+        "hr.qualification",
+        "private_info_id",
+        string="Language Qualification",
+        context={"active_test": False},
+        domain=[('qualification_category','=','language')]
+    )


My preference is to make these fields non-stored compute fields, like we did with capture_line_filtered_ids in project_task_capture, and keep the original qualification_ids as is, unless there is a technical limitation with this implementation for conceptual robustness. What makes me a bit uncomfortable with the current design is that these fields are added with dedicated table relations while they are needed merely for presentation purpose. But I might be overthinking...

Please disregard my previous comment; I think the current implementation is valid as having those xxx_qualification_ids fields doesn't affect the database structure.

[IMP] change char to many2one for qualification name field

7e72642

AungKoKoLin1997 requested review from yostashiro and kanda999 May 13, 2024 08:19

AungKoKoLin1997 changed the title ~~[4548][IMP] change char to many2one for qualification name field~~ [4548][IMP] l10n_jp_hr_employee: change char to many2one for qualification name field May 13, 2024

kanda999 reviewed May 13, 2024

View reviewed changes

add sequence

afaee7d

yostashiro reviewed May 13, 2024

View reviewed changes

AungKoKoLin1997 added 4 commits May 14, 2024 10:49

change model name

c167326

adj

1af99c0

adj

cd94df1

adj

3e05562

yostashiro reviewed May 17, 2024

View reviewed changes

AungKoKoLin1997 added 4 commits May 17, 2024 17:49

adj

b405325

adj

e325309

add force_save

b2ec82c

adj

63dc8b1

yostashiro reviewed May 17, 2024

View reviewed changes

AungKoKoLin1997 added 2 commits May 17, 2024 18:54

adj

e87da57

adj

936acc8

yostashiro reviewed May 17, 2024

View reviewed changes

yostashiro mentioned this pull request May 17, 2024

[4548][ADD] web_readonly_bypass qrtl/nrq-oca#13

Merged

AungKoKoLin1997 added 6 commits May 20, 2024 15:00

adj

1e2c17c

adj

ad5537e

adj

65cfcfd

adj

de1366d

adj

62a0117

adj

ee688ce

yostashiro approved these changes May 22, 2024

View reviewed changes

upd

b24420d

AungKoKoLin1997 requested a review from kanda999 May 24, 2024 08:14

AungKoKoLin1997 and others added 2 commits May 27, 2024 13:42

adj

c69e755

misc update

a6099b2

kanda999 force-pushed the 10.0-imp-l10n_jp_hr_employee branch from 05f9c26 to a6099b2 Compare May 27, 2024 07:25

kanda999 approved these changes May 27, 2024

View reviewed changes

kanda999 and others added 3 commits May 31, 2024 18:35

upd

5ee6dec

adj

9344226

adj

30312d9

AungKoKoLin1997 requested review from kanda999 and yostashiro June 4, 2024 09:14

kanda999 added 2 commits June 5, 2024 16:54

Adj translation

1d2c16b

misc update

b641e83

kanda999 force-pushed the 10.0-imp-l10n_jp_hr_employee branch from 4429b00 to b641e83 Compare June 5, 2024 09:38

yostashiro reviewed Jun 5, 2024

View reviewed changes

yostashiro approved these changes Jun 5, 2024

View reviewed changes

[IMP] hr_employee_nrq: adj qualification name fields

d70a411

AungKoKoLin1997 requested a review from yostashiro June 14, 2024 06:54

kanda999 added 3 commits June 17, 2024 11:18

add translation

6862ac6

misc update

72c303d

rmv qualification note

be9be57

kanda999 approved these changes Jun 20, 2024

View reviewed changes

kanda999 changed the title ~~[4548][IMP] l10n_jp_hr_employee: change char to many2one for qualification name field~~ [4548][IMP] l10n_jp_hr_employee, hr_employee_nrq, hr_employee_qualifications: change char to many2one for qualification name field Jun 20, 2024

kanda999 merged commit b4872cf into 10.0 Jun 20, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4548][IMP] l10n_jp_hr_employee, hr_employee_nrq, hr_employee_qualifications: change char to many2one for qualification name field #268

[4548][IMP] l10n_jp_hr_employee, hr_employee_nrq, hr_employee_qualifications: change char to many2one for qualification name field #268

AungKoKoLin1997 commented May 13, 2024 •

edited

Loading

kanda999 May 13, 2024

yostashiro May 13, 2024

AungKoKoLin1997 May 14, 2024

yostashiro left a comment

yostashiro May 17, 2024

yostashiro May 17, 2024

yostashiro May 17, 2024

yostashiro May 17, 2024

yostashiro May 17, 2024

yostashiro May 17, 2024 •

edited

Loading

AungKoKoLin1997 May 20, 2024

AungKoKoLin1997 May 21, 2024

yostashiro May 22, 2024

yostashiro left a comment

kanda999 left a comment

yostashiro Jun 5, 2024

yostashiro Jun 5, 2024 •

edited

Loading



		class QualificationName(models.Model):
		_name = "qualification.name"

		is_allowed_score = fields.Boolean(related="name.is_allowed_score", store=True)
		score = fields.Char()

-    is_allowed_score = fields.Boolean(related="name.is_allowed_score", store=True)
-    score = fields.Char()
+    qualification_type_id = fields.Many2one("hr.qualification.type", required=True, string="Qualification Type")
+    needs_description = fields.Boolean(related="qualification_type_id.needs_description")
+    description = fields.Char()

	name = fields.Many2one("hr.qualification.type", required=True)
	name = fields.Char(compute="_compute_name", store=True)

	needs_description = fields.Boolean(help="If enabled, allows input in the description field of the qualification.")
	needs_description = fields.Boolean(help="If enabled, description field of the qualification becomes required.")

[4548][IMP] l10n_jp_hr_employee, hr_employee_nrq, hr_employee_qualifications: change char to many2one for qualification name field #268

[4548][IMP] l10n_jp_hr_employee, hr_employee_nrq, hr_employee_qualifications: change char to many2one for qualification name field #268

Conversation

AungKoKoLin1997 commented May 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yostashiro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yostashiro May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yostashiro left a comment

Choose a reason for hiding this comment

kanda999 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yostashiro Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

AungKoKoLin1997 commented May 13, 2024 •

edited

Loading

yostashiro May 17, 2024 •

edited

Loading

yostashiro Jun 5, 2024 •

edited

Loading