Skip to content

Commit 30da2b1

Browse files
committed
fix(spp_import_match): multiple match error, missing field error, async context, jobworker
- Restore ValidationError on multiple matches instead of returning first - Add UserError when selected match config fields are missing from import file - Skip matching on empty field values to avoid false matches - Fix async import context: move import_match_ids to before sync/async branch - Simplify jobworker: rely on runner's 5-min auto-discovery instead of polling - Stop jobworker during resetdb to avoid DB lock issues - Remove forced jobworker restart from spp CLI start command
1 parent 2d636a6 commit 30da2b1

File tree

8 files changed

+59
-27
lines changed

8 files changed

+59
-27
lines changed

docker-compose.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ services:
170170

171171
# Job Worker - background service for processing async queue jobs
172172
# Runs alongside openspp/openspp-dev to execute delayed imports, etc.
173+
# The runner auto-discovers databases every 5 minutes, so after installing
174+
# new modules, jobs will be picked up within 5 minutes (or restart to force).
173175
jobworker:
174176
image: openspp-dev
175177
profiles:

spp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -726,8 +726,8 @@ def cmd_resetdb(args):
726726
# Ensure db is running
727727
run(docker_compose("up", "-d", "db"))
728728

729-
# Stop Odoo but keep db
730-
run(docker_compose("stop", "openspp", "openspp-dev"), check=False)
729+
# Stop Odoo and job worker but keep db
730+
run(docker_compose("stop", "openspp", "openspp-dev", "jobworker"), check=False)
731731

732732
# Wait for db to be ready
733733
print("Waiting for database...")

spp_import_match/README.rst

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ Key Capabilities
4646
Key Models
4747
~~~~~~~~~~
4848

49-
+-----------------------------+----------------------------------------+
50-
| Model | Description |
51-
+=============================+========================================+
52-
| ``spp.import.match`` | Matching rule configuration for a |
53-
| | specific model |
54-
+-----------------------------+----------------------------------------+
55-
| ``spp.import.match.fields`` | Individual field in a rule, with |
56-
| | optional sub-field |
57-
+-----------------------------+----------------------------------------+
49+
+-----------------------------+---------------------------------------+
50+
| Model | Description |
51+
+=============================+=======================================+
52+
| ``spp.import.match`` | Matching rule configuration for a |
53+
| | specific model |
54+
+-----------------------------+---------------------------------------+
55+
| ``spp.import.match.fields`` | Individual field in a rule, with |
56+
| | optional sub-field |
57+
+-----------------------------+---------------------------------------+
5858

5959
Configuration
6060
~~~~~~~~~~~~~

spp_import_match/models/base.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
import logging
33
import threading
44

5-
from odoo import api, models
5+
from odoo import _, api, models
6+
from odoo.exceptions import UserError
67

78
_logger = logging.getLogger(__name__)
89

@@ -19,11 +20,30 @@ def load(self, fields, data):
1920
if "import_match_ids" not in self.env.context:
2021
return super().load(fields, data)
2122

23+
import_match_ids = self.env.context.get("import_match_ids", [])
2224
usable, field_to_match = self.env["spp.import.match"]._usable_rules(
2325
self._name,
2426
fields,
25-
option_config_ids=self.env.context.get("import_match_ids", []),
27+
option_config_ids=import_match_ids,
2628
)
29+
30+
# Error if user selected match configs but their fields aren't in the file
31+
if import_match_ids and not usable:
32+
selected_configs = self.env["spp.import.match"].browse(import_match_ids)
33+
config_names = selected_configs.mapped("name")
34+
required_fields = set()
35+
for config in selected_configs:
36+
for field_line in config.field_ids:
37+
required_fields.add(field_line.field_id.field_description)
38+
raise UserError(
39+
_(
40+
"The selected import match configuration(s) %(configs)s require field(s) "
41+
"%(fields)s, but none of these fields are present in the imported file.",
42+
configs=", ".join(filter(None, config_names)),
43+
fields=", ".join(sorted(required_fields)),
44+
)
45+
)
46+
2747
# If overwrite_match is explicitly set via UI (context), use that.
2848
# Otherwise fall back to config records' overwrite_match setting.
2949
if "overwrite_match" in self.env.context:

spp_import_match/models/base_import.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,12 @@ def execute_import(self, fields, columns, options, dryrun=False):
6666
overwrite_match = options.get("overwrite_match", False)
6767
_import_match_local.counts = None
6868

69+
# Set import match context early so both sync and async paths have it
70+
if import_match_ids:
71+
self = self.with_context(import_match_ids=import_match_ids, overwrite_match=overwrite_match)
72+
6973
if dryrun or len(input_file_data) <= 100:
7074
_logger.info("Doing %s import", "dry-run" if dryrun else "normal")
71-
if import_match_ids:
72-
self = self.with_context(import_match_ids=import_match_ids, overwrite_match=overwrite_match)
7375
result = super().execute_import(fields, columns, options, dryrun=dryrun)
7476
counts = getattr(_import_match_local, "counts", None)
7577
if counts:

spp_import_match/models/import_match.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ def _match_find(self, model, converted_row, imported_row):
7373
break
7474
if field.field_id.name in converted_row:
7575
row_value = converted_row[field.field_id.name]
76+
# Skip matching on empty values to avoid false matches
77+
if not row_value:
78+
combination_valid = False
79+
break
7680
field_value = field.field_id.name
7781
add_to_domain = True
7882
if field.sub_field_id:
@@ -86,8 +90,12 @@ def _match_find(self, model, converted_row, imported_row):
8690
domain.append((field_value, "=", row_value))
8791
if not combination_valid:
8892
continue
93+
if not domain:
94+
continue
8995
match = model.search(domain)
90-
if len(match) >= 1:
96+
if len(match) > 1:
97+
raise ValidationError(_("Multiple records found for matching criteria: %s") % domain)
98+
if len(match) == 1:
9199
return match[0]
92100

93101
return model

spp_import_match/static/description/index.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,8 @@ <h1>Key Capabilities</h1>
395395
<h1>Key Models</h1>
396396
<table border="1" class="docutils">
397397
<colgroup>
398-
<col width="42%" />
399-
<col width="58%" />
398+
<col width="43%" />
399+
<col width="57%" />
400400
</colgroup>
401401
<thead valign="bottom">
402402
<tr><th class="head">Model</th>

spp_import_match/tests/test_import_match_model.py

Lines changed: 9 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_returns_first(self):
90-
"""Test _match_find returns first match when multiple found."""
91-
partner1 = self.env["res.partner"].create({"name": "DuplicateMatchTest"})
89+
def test_match_find_multiple_matches_raises(self):
90+
"""Test _match_find raises ValidationError when multiple matches found."""
91+
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-
result = match._match_find(
95-
self.env["res.partner"],
96-
{"name": "DuplicateMatchTest"},
97-
{"name": "DuplicateMatchTest", "id": None},
98-
)
99-
self.assertEqual(result, partner1)
94+
with self.assertRaises(ValidationError):
95+
match._match_find(
96+
self.env["res.partner"],
97+
{"name": "DuplicateMatchTest"},
98+
{"name": "DuplicateMatchTest", "id": None},
99+
)
100100

101101
def test_match_find_conditional_skip(self):
102102
"""Test _match_find skips rule when conditional value doesn't match."""

0 commit comments

Comments
 (0)