[MOD][18.0] hr_employee_language: languages selection#1517
[MOD][18.0] hr_employee_language: languages selection#1517
Conversation
2d3668b to
75eca3b
Compare
|
@dreispt @primes2h @TheMule71 plase review. |
|
@dreispt Thanks for your notes; I'm not so skilled on coding so your suggestion is very appreciated. I'll give a look asap. |
72cb747 to
356ce99
Compare
bfff99a to
ab2bd18
Compare
|
@dreispt Thanks to your comments I refactored the code. The only "strange" thing that I made is this: |
ab2bd18 to
87e51ec
Compare
|
@dreispt @primes2h @TheMule71 ping. Thanks |
1 similar comment
|
@dreispt @primes2h @TheMule71 ping. Thanks |
alexey-pelykh
left a comment
There was a problem hiding this comment.
Thanks for improving the language selection to use res.lang records. A couple of things to address:
-
_compute_display_name()searches ALLhr.employee.languagerecords (self.env["hr.employee.language"].search([])) instead of iterating overself. This is both a performance issue and semantically incorrect — compute methods should only set values onselfrecords. -
The compute method returns a list of tuples, but Odoo compute methods don't use return values — they assign to
self.display_name(which you do, but the return is dead code). -
Using
_()on selection keys (_("%(code)s", code=lang.code)) translates the code itself, which shouldn't be translated — codes are stable identifiers.
Happy to re-review once updated.
1b03217 to
c27e05b
Compare
c27e05b to
a3f5258
Compare
|
Thanks @alexey-pelykh for your suggestions. I updated the code, I hope in the right way. |
As in #1486
Changes the source of language list moving it from the res.lang.csv file to res.lang model.
In this way if you add a language at runtime it will be immediately available.
Added _compute_display_name (ex name_get) to show language name instead language code on selection or m2o widget