Skip to content

Commit

Permalink
discovery: report skipped entries
Browse files Browse the repository at this point in the history 
This makes it more obvious why certain components could not be created.

Fixes WeblateOrg#10515
  • Loading branch information
@nijel
nijel committed Dec 18, 2023
1 parent e54c49a commit 4b8a5f1
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 65 deletions.
1 change: 1 addition & 0 deletions docs/changes.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Not yet released.
**Improvements**

* Better logging in :wladmin:`createadmin`.
* :ref:`addon-weblate.discovery.discovery` now reports skipped entries.

**Bug fixes**

Expand Down
3 changes: 2 additions & 1 deletion weblate/addons/forms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def __init__(self, *args, **kwargs):
if self.cleaned_data.get("preview"):
self.fields["confirm"].widget = forms.CheckboxInput()
self.helper.layout.insert(0, Field("confirm"))
created, matched, deleted = self.discovery.perform(
created, matched, deleted, skipped = self.discovery.perform(
preview=True, remove=self.cleaned_data["remove"]
)
self.helper.layout.insert(
Expand All @@ -368,6 +368,7 @@ def __init__(self, *args, **kwargs):
"matches_created": created,
"matches_matched": matched,
"matches_deleted": deleted,
"matches_skipped": skipped,
"user": self.user,
},
),
Expand Down
106 changes: 70 additions & 36 deletions weblate/templates/addons/discovery_preview.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,51 +2,85 @@

<table class="table">
<thead>
<tr><th>{% trans "Component" %}</th><th>{% trans "Matched files" %}</th></tr>
<tr>
<th>{% trans "Component" %}</th>
<th></th>
<th>{% trans "Matched files" %}</th>
</tr>
</thead>
<tbody>
{% if matches_deleted %}
<tr><th colspan="3" class="danger">{% trans "The following components would be deleted" %}</th></tr>
{% for match in matches_deleted %}
<tr><th><a href="{{ match.1.get_absolute_url }}">{{ match.1 }}</a></th><td></td></tr>
{% endfor %}
<tr><th colspan="3" class="danger">{% trans "The following components would be deleted" %}</th></tr>
{% for match in matches_deleted %}
<tr>
<th><a href="{{ match.1.get_absolute_url }}">{{ match.1 }}</a></th>
<td></td>
<td></td>
</tr>
{% endfor %}
{% endif %}
{% if matches_created %}
<tr><th colspan="3" class="info">{% trans "The following components would be created" %}</th></tr>
{% for match in matches_created %}
<tr>
<th>{{ match.0.name }} (<code>{{ match.0.slug }}</code>)</th>
<td>
{% if match.0.base_file %}
<strong>{% trans "Monolingual base language file" %}:</strong> <code>{{ match.0.base_file }}</code><br/>
{% endif %}
<strong>{% trans "File mask" %}:</strong> <code>{{ match.0.mask }}</code><br/>
{% for file, lang in match.0.files_langs %}
<code>{{ file }}</code> ({{ lang }})<br/>
{% endfor %}
</td>
</tr>
{% endfor %}
<tr>
<th colspan="3" class="info">{% trans "The following components would be created" %}</th>
</tr>
{% for match in matches_created %}
<tr>
<th>{{ match.0.name }} (<code>{{ match.0.slug }}</code>)</th>
<td colspan="2">
{% if match.0.base_file %}
<strong>{% trans "Monolingual base language file" %}:</strong> <code>{{ match.0.base_file }}</code><br/>
{% endif %}
<strong>{% trans "File mask" %}:</strong> <code>{{ match.0.mask }}</code><br/>
{% for file, lang in match.0.files_langs %}
<code>{{ file }}</code> ({{ lang }})<br/>
{% endfor %}
</td>
</tr>
{% endfor %}
{% endif %}
{% if matches_matched %}
<tr><th colspan="3" class="info">{% trans "The following components matched existing ones" %}</th></tr>
{% for match in matches_matched %}
<tr>
<th><a href="{{ match.1.get_absolute_url }}">{{ match.1 }}</a></th>
<td>
{% if match.0.base_file %}
<strong>{% trans "Monolingual base language file" %}:</strong> <code>{{ match.0.base_file }}</code><br/>
<tr>
<th colspan="3" class="info">{% trans "The following components matched existing ones" %}</th>
</tr>
{% for match in matches_matched %}
<tr>
<th><a href="{{ match.1.get_absolute_url }}">{{ match.1 }}</a></th>
<td colspan="2">
{% if match.0.base_file %}
<strong>{% trans "Monolingual base language file" %}:</strong> <code>{{ match.0.base_file }}</code><br/>
{% endif %}
<strong>{% trans "File mask" %}:</strong> <code>{{ match.0.mask }}</code><br/>
{% for file, lang in match.0.files_langs %}
<code>{{ file }}</code> ({{ lang }})<br/>
{% endfor %}
</td>
</tr>
{% endfor %}
{% endif %}
<strong>{% trans "File mask" %}:</strong> <code>{{ match.0.mask }}</code><br/>
{% for file, lang in match.0.files_langs %}
<code>{{ file }}</code> ({{ lang }})<br/>
{% endfor %}
</td>
</tr>
{% endfor %}
{% if matches_skipped %}
<tr>
<th colspan="3" class="danger">{% trans "The following components could not be created" %}</th>
</tr>
{% for match in matches_skipped %}
<tr>
<th>{{ match.0.name }} (<code>{{ match.0.slug }}</code>)</th>
<td>
<strong>{{ match.1 }}</strong>
</td>
<td>
{% if match.0.base_file %}
<strong>{% trans "Monolingual base language file" %}:</strong> <code>{{ match.0.base_file }}</code><br/>
{% endif %}
<strong>{% trans "File mask" %}:</strong> <code>{{ match.0.mask }}</code><br/>
{% for file, lang in match.0.files_langs %}
<code>{{ file }}</code> ({{ lang }})<br/>
{% endfor %}
</td>
</tr>
{% endfor %}
{% endif %}
{% if not matches_deleted and not matches_created and not matches_matched %}
<tr><th colspan="3" class="danger">{% trans "No matching components were found!" %}</th></tr>
{% if not matches_deleted and not matches_created and not matches_matched and not matches_skipped %}
<tr><th colspan="3" class="danger">{% trans "No matching components were found!" %}</th></tr>
{% endif %}
</tbody>
</table>
Expand Down
38 changes: 19 additions & 19 deletions weblate/trans/discovery.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.db.models import Q
from django.utils.functional import cached_property
from django.utils.text import slugify
from django.utils.translation import gettext

from weblate.logger import LOGGER
from weblate.trans.defines import COMPONENT_NAME_LENGTH
Expand Down Expand Up @@ -267,39 +268,38 @@ def cleanup(self, main, processed, preview=False):

return deleted

def check_valid(self, match):
def valid_file(name):
if not name:
return True
fullname = os.path.join(self.component.full_path, name)
return os.path.exists(fullname)

def get_skip_reason(self, match):
# Skip matches to main component
if match["mask"] == self.component.filemask:
return False

if not valid_file(match["base_file"]):
return False

if not valid_file(match["new_base"]):
return False
return gettext("File mask matches the main component.")

if not valid_file(match["intermediate"]):
return False
for param in ("base_file", "new_base", "intermediate"):
name = match[param]
if not name:
continue
fullname = os.path.join(self.component.full_path, name)
if not os.path.exists(fullname):
return gettext("{filename} ({parameter}) does not exist.").format(
filename=name,
parameter=param,
)

return True
return None

def perform(self, preview=False, remove=False, background=False):
created = []
matched = []
deleted = []
skipped = []
processed = set()

main = self.component

for match in self.matched_components.values():
# Skip invalid matches
if not self.check_valid(match):
reason = self.get_skip_reason(match)
if reason is not None:
skipped.append((match, reason))
continue

try:
Expand All @@ -319,4 +319,4 @@ def perform(self, preview=False, remove=False, background=False):
if remove:
deleted = self.cleanup(main, processed, preview)

return created, matched, deleted
return created, matched, deleted, skipped
29 changes: 20 additions & 9 deletions weblate/trans/tests/test_discovery.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,27 @@ def test_matched_components(self):
def test_perform(self):
# Preview should not create anything
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = self.discovery.perform(preview=True)
created, matched, deleted, skipped = self.discovery.perform(preview=True)
self.assertEqual(len(created), 3)
self.assertEqual(len(matched), 0)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 1)

# Create components
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = self.discovery.perform()
created, matched, deleted, skipped = self.discovery.perform()
self.assertEqual(len(created), 3)
self.assertEqual(len(matched), 0)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 1)

# Test second call does nothing
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = self.discovery.perform()
created, matched, deleted, skipped = self.discovery.perform()
self.assertEqual(len(created), 0)
self.assertEqual(len(matched), 3)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 1)

# Remove some files
repository = self.component.repository
Expand All @@ -153,24 +156,29 @@ def test_perform(self):

# Test component removal preview
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = discovery.perform(preview=True, remove=True)
created, matched, deleted, skipped = discovery.perform(
preview=True, remove=True
)
self.assertEqual(len(created), 0)
self.assertEqual(len(matched), 1)
self.assertEqual(len(deleted), 2)
self.assertEqual(len(skipped), 1)

# Test component removal
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = discovery.perform(remove=True)
created, matched, deleted, skipped = discovery.perform(remove=True)
self.assertEqual(len(created), 0)
self.assertEqual(len(matched), 1)
self.assertEqual(len(deleted), 2)
self.assertEqual(len(skipped), 1)

# Components should be now removed
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = discovery.perform(remove=True)
created, matched, deleted, skipped = discovery.perform(remove=True)
self.assertEqual(len(created), 0)
self.assertEqual(len(matched), 1)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 1)

def test_duplicates(self):
# Create all components with desired name po
Expand All @@ -182,10 +190,11 @@ def test_duplicates(self):
file_format="po",
)
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = discovery.perform()
created, matched, deleted, skipped = discovery.perform()
self.assertEqual(len(created), 3)
self.assertEqual(len(matched), 0)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 1)

def test_multi_language(self):
discovery = ComponentDiscovery(
Expand All @@ -196,11 +205,12 @@ def test_multi_language(self):
file_format="po",
)
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = discovery.perform()
created, matched, deleted, skipped = discovery.perform()
self.assertEqual(len(created), 1)
self.assertEqual(created[0][0]["mask"], "localization/*/component.*.po")
self.assertEqual(len(matched), 0)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 0)

def test_named_group(self):
discovery = ComponentDiscovery(
Expand All @@ -211,10 +221,11 @@ def test_named_group(self):
file_format="po",
)
with override_settings(CREATE_GLOSSARIES=self.CREATE_GLOSSARIES):
created, matched, deleted = discovery.perform()
created, matched, deleted, skipped = discovery.perform()
self.assertEqual(len(created), 1)
self.assertEqual(created[0][0]["mask"], "localization/*/component.*.po")
self.assertEqual(created[0][0]["name"], "localization: component")
self.assertEqual(len(matched), 0)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(deleted), 0)
self.assertEqual(len(skipped), 0)

0 comments on commit 4b8a5f1

Please sign in to comment.