Skip to content

Commit 2d636a6

Browse files
committed
fix(spp_import_match): validation, default import behavior, and job worker
- Add @api.constrains for duplicate field and name validation on save - Multiple matches now return first match instead of raising error - Skip import matching when no config selected (use default Odoo import) - Add jobworker service to docker-compose for async import processing - Update tests to reflect new behavior
1 parent def5506 commit 2d636a6

File tree

6 files changed

+169
-43
lines changed

6 files changed

+169
-43
lines changed

docker-compose.yml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,44 @@ services:
168168
networks:
169169
- openspp
170170

171+
# Job Worker - background service for processing async queue jobs
172+
# Runs alongside openspp/openspp-dev to execute delayed imports, etc.
173+
jobworker:
174+
image: openspp-dev
175+
profiles:
176+
- dev
177+
- ui
178+
depends_on:
179+
db:
180+
condition: service_healthy
181+
environment:
182+
DB_HOST: db
183+
DB_PORT: "5432"
184+
DB_USER: odoo
185+
DB_PASSWORD: odoo
186+
DB_NAME: ${ODOO_DB_NAME:-openspp}
187+
DB_FILTER: "^${ODOO_DB_NAME:-openspp}$$"
188+
LIST_DB: "False"
189+
ODOO_ADMIN_PASSWD: admin
190+
ODOO_WORKERS: "0"
191+
ODOO_CRON_THREADS: "0"
192+
LOG_LEVEL: info
193+
PROXY_MODE: "False"
194+
QUEUE_JOB_CONCURRENCY: "2"
195+
volumes:
196+
- .:/mnt/extra-addons/openspp:ro,z
197+
- odoo_data:/var/lib/odoo
198+
entrypoint: ["/entrypoint.sh"]
199+
command:
200+
[
201+
"python",
202+
"/mnt/extra-addons/odoo-job-worker/job_worker_runner.py",
203+
"-c",
204+
"/etc/odoo/odoo.conf",
205+
]
206+
networks:
207+
- openspp
208+
171209
# Unit Test Runner - one-off container for running module tests
172210
# Usage: docker compose run --rm test <module_name> [options]
173211
# Note: Uses same image as odoo/odoo-dev - build any of them to update

spp_import_match/models/base.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ class Base(models.AbstractModel):
1515

1616
@api.model
1717
def load(self, fields, data):
18+
# Only apply import matching when explicitly selected via UI context
19+
if "import_match_ids" not in self.env.context:
20+
return super().load(fields, data)
21+
1822
usable, field_to_match = self.env["spp.import.match"]._usable_rules(
1923
self._name,
2024
fields,

spp_import_match/models/import_match.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,26 @@ def _onchange_model_id(self):
3939
for rec in self:
4040
rec.field_ids = None
4141

42+
@api.constrains("name")
43+
def _check_duplicate_name(self):
44+
"""Prevent duplicate match rule names."""
45+
for rec in self:
46+
if rec.name and self.search_count([("name", "=", rec.name), ("id", "!=", rec.id)]):
47+
raise ValidationError(_("A match rule with the name '%s' already exists!") % rec.name)
48+
49+
@api.constrains("field_ids")
50+
def _check_duplicate_fields(self):
51+
"""Prevent duplicate non-relational fields in the same match rule."""
52+
for rec in self:
53+
seen = []
54+
for field_line in rec.field_ids:
55+
if field_line.field_id.ttype in ("many2many", "one2many", "many2one"):
56+
continue
57+
key = field_line.field_id.id
58+
if key in seen:
59+
raise ValidationError(_("Field '%s', already exists!") % field_line.field_id.field_description)
60+
seen.append(key)
61+
4262
@api.model
4363
def _match_find(self, model, converted_row, imported_row):
4464
usable, field_to_match = self._usable_rules(model._name, converted_row)
@@ -67,10 +87,8 @@ def _match_find(self, model, converted_row, imported_row):
6787
if not combination_valid:
6888
continue
6989
match = model.search(domain)
70-
if len(match) == 1:
71-
return match
72-
elif len(match) > 1:
73-
raise ValidationError(_("Multiple matches found for '%s'!") % match[0].name)
90+
if len(match) >= 1:
91+
return match[0]
7492

7593
return model
7694

spp_import_match/tests/test_base_load.py

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,25 @@ def _create_match_rule(self, field_ids_data, overwrite=True):
3333
self.env["spp.import.match.fields"].create(data)
3434
return match
3535

36-
def test_load_no_usable_rules(self):
37-
"""When no usable rules exist, load() passes through to super()."""
36+
def test_load_no_context_passes_through(self):
37+
"""When import_match_ids not in context, load() passes through to default Odoo import."""
3838
result = self.env["res.partner"].load(
3939
["name", "email"],
40-
[["NoRuleTest99xyz", "norule@test.com"]],
40+
[["NoCtxTest99xyz", "noctx@test.com"]],
41+
)
42+
self.assertFalse(result["messages"])
43+
partner = self.env["res.partner"].search([("name", "=", "NoCtxTest99xyz")])
44+
self.assertEqual(len(partner), 1)
45+
46+
def test_load_no_usable_rules(self):
47+
"""When import_match_ids in context but no usable rules, load() passes through."""
48+
result = (
49+
self.env["res.partner"]
50+
.with_context(import_match_ids=[])
51+
.load(
52+
["name", "email"],
53+
[["NoRuleTest99xyz", "norule@test.com"]],
54+
)
4155
)
4256
self.assertFalse(result["messages"])
4357
partner = self.env["res.partner"].search([("name", "=", "NoRuleTest99xyz")])
@@ -46,10 +60,14 @@ def test_load_no_usable_rules(self):
4660
def test_load_match_overwrite_true(self):
4761
"""Match found + overwrite=True -> record updated."""
4862
partner = self.env["res.partner"].create({"name": "OverwriteTrue99xyz", "email": "old@test.com"})
49-
self._create_match_rule([{"field_id": self.name_field.id}], overwrite=True)
50-
result = self.env["res.partner"].load(
51-
["name", "email"],
52-
[["OverwriteTrue99xyz", "new@test.com"]],
63+
match = self._create_match_rule([{"field_id": self.name_field.id}], overwrite=True)
64+
result = (
65+
self.env["res.partner"]
66+
.with_context(import_match_ids=[match.id])
67+
.load(
68+
["name", "email"],
69+
[["OverwriteTrue99xyz", "new@test.com"]],
70+
)
5371
)
5472
self.assertFalse(result["messages"])
5573
partner.invalidate_recordset()
@@ -58,20 +76,28 @@ def test_load_match_overwrite_true(self):
5876
def test_load_match_overwrite_false(self):
5977
"""Match found + overwrite=False -> record skipped."""
6078
partner = self.env["res.partner"].create({"name": "OverwriteFalse99xyz", "email": "original@test.com"})
61-
self._create_match_rule([{"field_id": self.name_field.id}], overwrite=False)
62-
self.env["res.partner"].load(
63-
["name", "email"],
64-
[["OverwriteFalse99xyz", "changed@test.com"]],
79+
match = self._create_match_rule([{"field_id": self.name_field.id}], overwrite=False)
80+
(
81+
self.env["res.partner"]
82+
.with_context(import_match_ids=[match.id])
83+
.load(
84+
["name", "email"],
85+
[["OverwriteFalse99xyz", "changed@test.com"]],
86+
)
6587
)
6688
partner.invalidate_recordset()
6789
self.assertEqual(partner.email, "original@test.com")
6890

6991
def test_load_no_match_creates_record(self):
7092
"""No match found -> new record created."""
71-
self._create_match_rule([{"field_id": self.name_field.id}])
72-
result = self.env["res.partner"].load(
73-
["name", "email"],
74-
[["BrandNewPartner99xyz", "brandnew@test.com"]],
93+
match = self._create_match_rule([{"field_id": self.name_field.id}])
94+
result = (
95+
self.env["res.partner"]
96+
.with_context(import_match_ids=[match.id])
97+
.load(
98+
["name", "email"],
99+
[["BrandNewPartner99xyz", "brandnew@test.com"]],
100+
)
75101
)
76102
self.assertFalse(result["messages"])
77103
partner = self.env["res.partner"].search([("name", "=", "BrandNewPartner99xyz")])
@@ -81,13 +107,17 @@ def test_load_no_match_creates_record(self):
81107
def test_load_multiple_records_mixed(self):
82108
"""Mix of matched and new records in one import."""
83109
self.env["res.partner"].create({"name": "ExistingMixed99xyz", "email": "existing@test.com"})
84-
self._create_match_rule([{"field_id": self.name_field.id}], overwrite=True)
85-
result = self.env["res.partner"].load(
86-
["name", "email"],
87-
[
88-
["ExistingMixed99xyz", "updated@test.com"],
89-
["NewMixed99xyz", "newmixed@test.com"],
90-
],
110+
match = self._create_match_rule([{"field_id": self.name_field.id}], overwrite=True)
111+
result = (
112+
self.env["res.partner"]
113+
.with_context(import_match_ids=[match.id])
114+
.load(
115+
["name", "email"],
116+
[
117+
["ExistingMixed99xyz", "updated@test.com"],
118+
["NewMixed99xyz", "newmixed@test.com"],
119+
],
120+
)
91121
)
92122
self.assertFalse(result["messages"])
93123
existing = self.env["res.partner"].search([("name", "=", "ExistingMixed99xyz")])
@@ -117,11 +147,15 @@ def test_load_with_import_match_ids_context(self):
117147

118148
def test_load_appends_id_field(self):
119149
"""Auto-appends 'id' when not in fields list."""
120-
self._create_match_rule([{"field_id": self.name_field.id}])
150+
match = self._create_match_rule([{"field_id": self.name_field.id}])
121151
fields = ["name", "email"]
122-
self.env["res.partner"].load(
123-
fields,
124-
[["AppendIdTest99xyz", "appendid@test.com"]],
152+
(
153+
self.env["res.partner"]
154+
.with_context(import_match_ids=[match.id])
155+
.load(
156+
fields,
157+
[["AppendIdTest99xyz", "appendid@test.com"]],
158+
)
125159
)
126160
self.assertIn("id", fields)
127161

spp_import_match/tests/test_import_match_model.py

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,17 @@ def test_match_find_no_match(self):
8686
# Should return the model (empty recordset)
8787
self.assertFalse(result.id)
8888

89-
def test_match_find_multiple_matches_raises(self):
90-
"""Test _match_find raises ValidationError on multiple matches."""
91-
self.env["res.partner"].create({"name": "DuplicateMatchTest"})
89+
def test_match_find_multiple_matches_returns_first(self):
90+
"""Test _match_find returns first match when multiple found."""
91+
partner1 = self.env["res.partner"].create({"name": "DuplicateMatchTest"})
9292
self.env["res.partner"].create({"name": "DuplicateMatchTest"})
9393
match = self._create_match_rule([{"field_id": self.name_field.id}])
94-
with self.assertRaises(ValidationError):
95-
match._match_find(
96-
self.env["res.partner"],
97-
{"name": "DuplicateMatchTest"},
98-
{"name": "DuplicateMatchTest", "id": None},
99-
)
94+
result = match._match_find(
95+
self.env["res.partner"],
96+
{"name": "DuplicateMatchTest"},
97+
{"name": "DuplicateMatchTest", "id": None},
98+
)
99+
self.assertEqual(result, partner1)
100100

101101
def test_match_find_conditional_skip(self):
102102
"""Test _match_find skips rule when conditional value doesn't match."""
@@ -211,6 +211,36 @@ def test_match_find_sub_field(self):
211211
)
212212
self.assertEqual(result, parent)
213213

214+
def test_constrains_duplicate_name(self):
215+
"""_check_duplicate_name raises on duplicate rule names."""
216+
self.env["spp.import.match"].create({"name": "DuplicateNameTest", "model_id": self.res_partner_model.id})
217+
with self.assertRaises(ValidationError):
218+
self.env["spp.import.match"].create({"name": "DuplicateNameTest", "model_id": self.res_partner_model.id})
219+
220+
def test_constrains_duplicate_non_relational_field(self):
221+
"""_check_duplicate_fields raises on duplicate non-relational fields on save."""
222+
with self.assertRaises(ValidationError):
223+
self._create_match_rule(
224+
[
225+
{"field_id": self.name_field.id},
226+
{"field_id": self.name_field.id},
227+
]
228+
)
229+
230+
def test_constrains_allows_duplicate_relational_field(self):
231+
"""_check_duplicate_fields allows duplicate relational fields."""
232+
parent_id_field = self.env["ir.model.fields"].search(
233+
[("name", "=", "parent_id"), ("model_id", "=", self.res_partner_model.id)],
234+
limit=1,
235+
)
236+
match = self._create_match_rule(
237+
[
238+
{"field_id": parent_id_field.id},
239+
{"field_id": parent_id_field.id},
240+
]
241+
)
242+
self.assertEqual(len(match.field_ids), 2)
243+
214244
def test_match_find_multiple_combinations(self):
215245
"""_match_find iterates rules; first matching single result wins."""
216246
partner = self.env["res.partner"].create({"name": "MultiCombo99xyz", "email": "multicombo@test.com"})

spp_import_match/tests/test_res_partner_import_match.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,23 @@ def create_matching_name(self):
110110

111111
def test_01_res_partner_change_email_by_name(self):
112112
"""Change email based on given_name, family_name."""
113-
self.create_matching_given_family_name()
113+
import_match = self.create_matching_given_family_name()
114114
file_path = self.get_file_path_2()
115115
record = self._base_import_record("res.partner", file_path)
116-
record.execute_import(["given_name", "family_name", "name", "email"], [], OPTIONS)
116+
options = dict(OPTIONS, import_match_ids=[import_match.id], overwrite_match=True)
117+
record.execute_import(["given_name", "family_name", "name", "email"], [], options)
117118

118119
self._test_applicant.env.cache.invalidate()
119120
self.assertEqual(self._test_applicant.email, "rufinorenaud@gmail.com")
120121

121122
def test_02_res_partner_change_email_by_group_name(self):
122123
"""Change email based on name."""
123-
self.create_matching_name()
124+
import_match = self.create_matching_name()
124125
file_path = self.get_file_path_1()
125126
record = self._base_import_record("res.partner", file_path)
126127

127-
record.execute_import(["name", "email"], ["name", "email"], OPTIONS)
128+
options = dict(OPTIONS, import_match_ids=[import_match.id], overwrite_match=True)
129+
record.execute_import(["name", "email"], ["name", "email"], options)
128130
self._test_hh.env.cache.invalidate()
129131
self.assertEqual(self._test_hh.email, "renaudhh@gmail.com")
130132

0 commit comments

Comments
 (0)