actioned tickets 2482 and 2609 - extra carousel image check and base assets incorrectly flagged as wrong file type
This commit is contained in:
michael
parent
d63b671b01
commit
7b4a338665
3 changed files with 191 additions and 29 deletions
|
|
@ -193,17 +193,16 @@ def validate_record_features(record: Dict[str, Any], working_dir: str, item_inde
|
|||
}
|
||||
|
||||
# Check each feature for matching images using substring search
|
||||
# Only match files containing 'extra' to avoid false matches with MEC/BAU backgrounds
|
||||
matched_features = []
|
||||
found_filenames = []
|
||||
missing_features = []
|
||||
|
||||
for feature in features:
|
||||
# Case-sensitive substring search in filenames
|
||||
matching_files = [f for f in available_files if feature in f]
|
||||
# Case-sensitive substring search in filenames containing 'extra'
|
||||
matching_files = [f for f in available_files if feature in f and 'extra' in f.lower()]
|
||||
|
||||
if matching_files:
|
||||
matched_features.append(feature)
|
||||
found_filenames.extend(matching_files)
|
||||
else:
|
||||
missing_features.append(feature)
|
||||
|
||||
|
|
@ -214,11 +213,11 @@ def validate_record_features(record: Dict[str, Any], working_dir: str, item_inde
|
|||
"status": "success",
|
||||
"record_index": record_index,
|
||||
"item_index": item_index,
|
||||
"features": features,
|
||||
"total_features": len(features),
|
||||
"coverage": round(coverage, 1),
|
||||
"features": features, # Include full features list for aggregation
|
||||
"matched_features": matched_features,
|
||||
"missing_features": missing_features,
|
||||
"found_filenames": sorted(list(set(found_filenames))) # Remove duplicates and sort
|
||||
"missing_features": missing_features
|
||||
}
|
||||
|
||||
def check_feature_in_filename(feature: str, filename: str) -> bool:
|
||||
|
|
|
|||
|
|
@ -301,6 +301,9 @@ class HTMLReporter:
|
|||
formatted_parts.append(HTMLReporter._format_business_data(details))
|
||||
elif 'unreferenced_files' in details:
|
||||
formatted_parts.append(HTMLReporter._format_file_list(details['unreferenced_files'], "Unreferenced Files"))
|
||||
elif 'record_results' in details and 'overall_coverage' in details:
|
||||
# Extra carousel validation check
|
||||
formatted_parts.append(HTMLReporter._format_extra_carousel_validation(details))
|
||||
# If no special formatting applied so far, use generic formatting
|
||||
elif not formatted_parts:
|
||||
formatted_parts.append(f'''
|
||||
|
|
@ -1302,6 +1305,107 @@ class HTMLReporter:
|
|||
<pre>{json.dumps(details, indent=2, default=str)}</pre>
|
||||
'''
|
||||
|
||||
@staticmethod
|
||||
def _format_extra_carousel_validation(details: dict) -> str:
|
||||
"""
|
||||
Format extra carousel validation results with focus on problems only.
|
||||
Shows overall summary, brief passed records, and detailed failed records.
|
||||
"""
|
||||
try:
|
||||
if not details:
|
||||
return '<div class="alert alert-info">No details available</div>'
|
||||
|
||||
message = details.get('message', 'Extra carousel validation completed')
|
||||
pack_type = details.get('pack_type', 'Unknown')
|
||||
overall_coverage = details.get('overall_coverage', 0)
|
||||
total_features = details.get('total_features_across_all_records', 0)
|
||||
matched_count = details.get('matched_features_count', 0)
|
||||
record_results = details.get('record_results', [])
|
||||
missing_features_summary = details.get('missing_features_summary', [])
|
||||
|
||||
# Separate passed and failed records
|
||||
passed_records = [r for r in record_results if r.get('coverage', 0) == 100]
|
||||
failed_records = [r for r in record_results if r.get('coverage', 0) < 100]
|
||||
|
||||
html_parts = []
|
||||
|
||||
# Overall summary section
|
||||
html_parts.append(f'''
|
||||
<div class="alert alert-info mb-3">
|
||||
<h6 class="alert-heading">Overall Summary</h6>
|
||||
<ul class="mb-0">
|
||||
<li><strong>Pack Type:</strong> {pack_type}</li>
|
||||
<li><strong>Coverage:</strong> {matched_count}/{total_features} features matched ({overall_coverage}%)</li>
|
||||
<li><strong>Status:</strong> {message}</li>
|
||||
</ul>
|
||||
</div>
|
||||
''')
|
||||
|
||||
# Passed records - brief summary
|
||||
if passed_records:
|
||||
html_parts.append(f'''
|
||||
<div class="mb-3">
|
||||
<h6 class="text-success">✓ Passed Records ({len(passed_records)})</h6>
|
||||
<ul class="list-unstyled">
|
||||
{''.join(f'<li class="text-muted">✓ Record {r.get("record_index", "?")} (Item {r.get("item_index", "?")}): 100% coverage ({r.get("total_features", 0)} features)</li>' for r in passed_records)}
|
||||
</ul>
|
||||
</div>
|
||||
''')
|
||||
|
||||
# Failed records - detailed accordion
|
||||
if failed_records:
|
||||
html_parts.append(f'''
|
||||
<div class="mb-3">
|
||||
<h6 class="text-danger">✗ Failed Records ({len(failed_records)})</h6>
|
||||
<div class="accordion" id="failedRecordsAccordion">
|
||||
{''.join(f"""
|
||||
<div class="accordion-item">
|
||||
<h2 class="accordion-header" id="heading-record-{i}">
|
||||
<button class="accordion-button {'collapsed' if i > 0 else ''}" type="button"
|
||||
data-bs-toggle="collapse"
|
||||
data-bs-target="#collapse-record-{i}"
|
||||
aria-expanded="{'true' if i == 0 else 'false'}"
|
||||
aria-controls="collapse-record-{i}">
|
||||
Record {record.get('record_index', '?')} (Item {record.get('item_index', '?')}): {record.get('coverage', 0)}% coverage - Missing {len(record.get('missing_features', []))} WERS codes
|
||||
</button>
|
||||
</h2>
|
||||
<div id="collapse-record-{i}"
|
||||
class="accordion-collapse collapse {'show' if i == 0 else ''}"
|
||||
aria-labelledby="heading-record-{i}">
|
||||
<div class="accordion-body">
|
||||
<p><strong>Coverage:</strong> {len(record.get('matched_features', []))}/{record.get('total_features', 0)} features matched ({record.get('coverage', 0)}%)</p>
|
||||
{f'<p><strong>Missing WERS Codes:</strong></p><ul>{"".join(f"<li><code>{wers}</code></li>" for wers in record.get("missing_features", []))}</ul>' if record.get('missing_features') else '<p class="text-success">No missing features</p>'}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
""" for i, record in enumerate(failed_records))}
|
||||
</div>
|
||||
</div>
|
||||
''')
|
||||
|
||||
# Missing features summary (if applicable)
|
||||
if missing_features_summary:
|
||||
html_parts.append(f'''
|
||||
<div class="alert alert-warning mb-3">
|
||||
<h6 class="alert-heading">All Missing WERS Codes Across Records</h6>
|
||||
<p class="mb-1">The following WERS codes are missing from extra carousel images:</p>
|
||||
<div class="d-flex flex-wrap gap-2">
|
||||
{''.join(f'<code class="badge bg-warning text-dark">{wers}</code>' for wers in missing_features_summary)}
|
||||
</div>
|
||||
</div>
|
||||
''')
|
||||
|
||||
return ''.join(html_parts)
|
||||
|
||||
except Exception as e:
|
||||
logging.warning(f"Error formatting extra carousel validation: {e}")
|
||||
return f'''
|
||||
<div class="alert alert-warning">
|
||||
Error formatting extra carousel validation: {str(e)}
|
||||
</div>
|
||||
<pre>{json.dumps(details, indent=2, default=str)}</pre>
|
||||
'''
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
import sys
|
||||
|
|
|
|||
|
|
@ -55,11 +55,12 @@ def run_check(config):
|
|||
('exterior', 'layeroptext'): 'PNG',
|
||||
('interior', 'layeroptint'): 'PNG',
|
||||
('carousel', 'extra'): 'JPEG',
|
||||
('carousel', 'powertrain'): 'JPEG',
|
||||
('carousel', 'powertrain'): 'JPEG', # BAU powertrain only
|
||||
('exterior', 'showroom'): 'PNG',
|
||||
('carousel', 'colour'): 'JPEG',
|
||||
('carousel', 'bodystyle'): 'JPEG',
|
||||
('carousel', 'series'): 'JPEG',
|
||||
('exterior', 'series'): 'JPEG', # Per latest guidance (newer format)
|
||||
('carousel', 'series'): 'JPEG', # Legacy format (older packs)
|
||||
('carousel', 'trim'): 'JPEG',
|
||||
# Adding lifestyle and inventory requirements
|
||||
('lifestyle', None): 'JPEG',
|
||||
|
|
@ -73,19 +74,42 @@ def run_check(config):
|
|||
"""Check if this is a base exterior or interior asset."""
|
||||
return (viewtype == 'exterior' and imagetype is None) or (viewtype == 'interior' and imagetype is None)
|
||||
|
||||
def validate_base_asset_format(filename, actual_format, pack_type, viewtype):
|
||||
def is_mec_powertrain(viewtype, imagetype, conditions):
|
||||
"""
|
||||
Check if this is a MEC powertrain image.
|
||||
MEC powertrains are identified by:
|
||||
- viewtype="exterior"
|
||||
- imagetype=None
|
||||
- experienceCondition="2d-background"
|
||||
- angle=30
|
||||
"""
|
||||
return (
|
||||
viewtype == 'exterior' and
|
||||
imagetype is None and
|
||||
conditions.get('experienceCondition') == '2d-background' and
|
||||
conditions.get('angle') == 30
|
||||
)
|
||||
|
||||
def validate_base_asset_format(filename, actual_format, asset_type, viewtype):
|
||||
"""
|
||||
Validate base asset format with flexible rules:
|
||||
- MEC base assets: Strict JPG only (fail if PNG)
|
||||
- BAU base assets: JPG preferred, PNG acceptable with warning
|
||||
- MEC base assets (with experienceCondition="2d-background"): Strict JPG only (fail if PNG)
|
||||
- BAU base assets (without experienceCondition): JPG preferred, PNG acceptable with warning
|
||||
|
||||
Args:
|
||||
filename: The image filename
|
||||
actual_format: The detected image format (e.g., "JPEG", "PNG")
|
||||
asset_type: "MEC" or "BAU" - indicates if this specific asset has experienceCondition
|
||||
viewtype: The viewtype of the asset (e.g., "exterior", "interior")
|
||||
|
||||
Returns: (is_valid, warning_message)
|
||||
"""
|
||||
if pack_type == "MEC":
|
||||
if asset_type == "MEC":
|
||||
# MEC base assets must be JPG only
|
||||
if actual_format != "JPEG":
|
||||
return False, None
|
||||
return True, None
|
||||
else: # BAU pack
|
||||
else: # BAU asset
|
||||
# BAU base assets: JPG preferred, PNG acceptable with warning
|
||||
if actual_format == "JPEG":
|
||||
return True, None
|
||||
|
|
@ -111,14 +135,31 @@ def run_check(config):
|
|||
logging.debug(f"Skipping item with missing viewtype: {conditions}")
|
||||
continue
|
||||
|
||||
# Check if this is a base asset that needs special handling
|
||||
if is_base_asset(viewtype, imagetype):
|
||||
# Base assets use flexible validation logic
|
||||
expected_format = None # Will be handled in the validation function
|
||||
# Determine asset type and validation strategy
|
||||
is_mec_powertrain_asset = False
|
||||
is_base_asset_flag = False
|
||||
base_asset_type = None
|
||||
expected_format = None
|
||||
|
||||
# Check 1: Is this a MEC powertrain? (must check before base asset check)
|
||||
if is_mec_powertrain(viewtype, imagetype, conditions):
|
||||
is_mec_powertrain_asset = True
|
||||
expected_format = "JPEG"
|
||||
|
||||
# Check 2: Is this a base asset? (exterior/interior with no imagetype)
|
||||
elif is_base_asset(viewtype, imagetype):
|
||||
is_base_asset_flag = True
|
||||
# Determine if this specific base asset is MEC or BAU version
|
||||
# MEC version has experienceCondition="2d-background" (but not angle=30)
|
||||
if conditions.get('experienceCondition') == '2d-background':
|
||||
base_asset_type = "MEC"
|
||||
else:
|
||||
base_asset_type = "BAU"
|
||||
# expected_format will be handled in the validation function
|
||||
|
||||
# Check 3: Regular assets - look up in format requirements
|
||||
else:
|
||||
# Non-base assets use the standard format requirements
|
||||
key = (viewtype, imagetype)
|
||||
expected_format = None
|
||||
|
||||
if key in format_requirements:
|
||||
expected_format = format_requirements[key]
|
||||
|
|
@ -146,7 +187,7 @@ def run_check(config):
|
|||
|
||||
# Check actual image format
|
||||
# Special handling for AVIF files - use extension validation since PIL may not support them
|
||||
if not is_base_asset(viewtype, imagetype) and expected_format == "AVIF":
|
||||
if not is_base_asset_flag and expected_format == "AVIF":
|
||||
# For AVIF files, validate by extension instead of PIL
|
||||
if filename.lower().endswith('.avif'):
|
||||
# AVIF file with correct extension - pass validation
|
||||
|
|
@ -168,21 +209,21 @@ def run_check(config):
|
|||
with Image.open(image_path) as img:
|
||||
actual_format = img.format
|
||||
|
||||
if is_base_asset(viewtype, imagetype):
|
||||
if is_base_asset_flag:
|
||||
# Use flexible validation for base assets
|
||||
is_valid, warning_message = validate_base_asset_format(filename, actual_format, pack_type, viewtype)
|
||||
is_valid, warning_message = validate_base_asset_format(filename, actual_format, base_asset_type, viewtype)
|
||||
|
||||
if not is_valid:
|
||||
# Only add an entry if this filename not yet in the dict
|
||||
if filename not in failed_images_dict:
|
||||
expected_desc = "JPEG" if pack_type == "MEC" else "JPEG (preferred) or PNG (acceptable)"
|
||||
expected_desc = "JPEG" if base_asset_type == "MEC" else "JPEG (preferred) or PNG (acceptable)"
|
||||
failed_images_dict[filename] = {
|
||||
"filename": filename,
|
||||
"viewtype": viewtype,
|
||||
"imagetype": imagetype,
|
||||
"imagetype": imagetype if imagetype else "base",
|
||||
"expected_format": expected_desc,
|
||||
"actual_format": actual_format,
|
||||
"pack_type": pack_type
|
||||
"asset_type": base_asset_type
|
||||
}
|
||||
elif warning_message:
|
||||
# Add warning but don't fail
|
||||
|
|
@ -190,8 +231,19 @@ def run_check(config):
|
|||
"filename": filename,
|
||||
"message": warning_message
|
||||
})
|
||||
elif is_mec_powertrain_asset:
|
||||
# Validate MEC powertrain format (strict JPEG)
|
||||
if actual_format != expected_format:
|
||||
if filename not in failed_images_dict:
|
||||
failed_images_dict[filename] = {
|
||||
"filename": filename,
|
||||
"viewtype": viewtype,
|
||||
"imagetype": "powertrain (MEC)",
|
||||
"expected_format": expected_format,
|
||||
"actual_format": actual_format
|
||||
}
|
||||
else:
|
||||
# Standard validation for non-base assets
|
||||
# Standard validation for regular assets
|
||||
# PIL reports "JPEG" for JPG files
|
||||
if actual_format != expected_format:
|
||||
# Only add an entry if this filename not yet in the dict
|
||||
|
|
@ -207,11 +259,18 @@ def run_check(config):
|
|||
except Exception as e:
|
||||
# If we can't open the image, mark it as failed
|
||||
if filename not in failed_images_dict:
|
||||
expected_desc = expected_format if expected_format else ("JPEG" if pack_type == "MEC" and is_base_asset(viewtype, imagetype) else "Unknown")
|
||||
# Determine expected description based on asset type
|
||||
if is_base_asset_flag:
|
||||
expected_desc = "JPEG" if base_asset_type == "MEC" else "JPEG (preferred) or PNG (acceptable)"
|
||||
elif expected_format:
|
||||
expected_desc = expected_format
|
||||
else:
|
||||
expected_desc = "Unknown"
|
||||
|
||||
failed_images_dict[filename] = {
|
||||
"filename": filename,
|
||||
"viewtype": viewtype,
|
||||
"imagetype": imagetype,
|
||||
"imagetype": imagetype if imagetype else ("base" if is_base_asset_flag else None),
|
||||
"expected_format": expected_desc,
|
||||
"actual_format": f"Error: {str(e)}"
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue