Invoice designer module

[mborchuk]

No description provided.

To fix the problem, the duplicated key in the labels dictionary should be removed so that each key appears only once. Since both 'total' entries map to the same string 'Total', the safest, behavior‑preserving fix is simply to delete one of the duplicate lines and keep the other.

Concretely, in modules/invoice_designer/index.py, inside the DEFAULT_CONFIG dict, under the 'labels' sub-dictionary, remove one of the two 'total': 'Total', lines. The minimal and clearest change is to delete the earlier occurrence on line 67 and retain the later one on line 70, or vice versa; either choice yields identical runtime behavior. No new methods, imports, or definitions are required; this is just a cleanup of the literal.

In general, to fix untrusted URL redirection you should avoid redirecting to URLs that are derived directly from the incoming request or user input. Instead, redirect to a fixed internal endpoint (e.g. url_for('invoice_designer.designer_index') or the current view’s URL), or validate that the redirect target is a safe, relative URL without an external host or scheme.

For this code, the minimal and safest fix that doesn’t change intended functionality is:

• Replace redirect(request.url) with a redirect to the current designer page using url_for.
• In both places where request.url is used (line 316 for missing name, and line 416 in the exception handler), route the user back to the designer index page instead of echoing the full incoming URL.

The underlying behavior remains essentially the same: on validation failure or error, the user is taken back to the designer UI. But we break the direct dependency on a potentially tainted URL. No new helpers or imports are needed because url_for and redirect are already imported at line 20.

Concretely:

• In modules/invoice_designer/index.py, inside _save_template, change:
  • Line 316: return redirect(request.url) → return redirect(url_for('invoice_designer.designer_index'))
  • Line 416: return redirect(request.url) → return redirect(url_for('invoice_designer.designer_index'))

In general, the issue is fixed by not using request.url (which is derived from the incoming request, including user-controlled pieces) directly as a redirect target. Instead, redirect only to fixed, server-defined routes (for example using url_for) or to rigorously validated relative paths.

In this file, request.url is only used in _save_template when a validation error occurs (missing template name) and when an exception is caught. In both cases, we conceptually want to send the user back to the template form. Rather than redirecting to whatever URL was requested, we should redirect to the explicit create or edit endpoints. We can distinguish these cases using the tpl argument:

• If tpl is None, we are creating a template, so redirect to the create route (e.g., invoice_designer.designer_create).
• If tpl is not None, we are editing an existing template, so redirect to the edit route invoice_designer.designer_edit, passing id=tpl.id.

Concretely:

• Replace redirect(request.url) at line 316 with a conditional redirect to the appropriate route via url_for.
• Similarly, replace redirect(request.url) at line 416 in the except block with the same conditional redirect logic.
• Use the already-imported url_for and redirect from Flask; no new imports are needed.
• Keep the surrounding behavior (flashes, DB rollback) unchanged.

In general, empty except blocks should either be removed (letting exceptions propagate) or augmented to handle the error explicitly, typically by logging, transforming, or otherwise reacting to the exception. When silent degradation is desired, logging at an appropriate level is usually the minimum acceptable handling.

For this specific case in modules/invoice_designer/index.py, the best fix that does not change functional behavior is to keep the broad except Exception: (so the method still returns [] on failure) but log the exception. The file already imports logging and defines logger, so we just need to call logger.exception(...) or logger.warning(...) with a clear message in the except block. This introduces no new dependencies and preserves the external interface of get_invoice_templates.

Concretely, in get_invoice_templates, replace the except Exception: pass block (lines 280–281) with an except Exception: block that logs the error using logger.exception("..."). No other files or imports need modification.

In general, an empty except should be replaced with either more specific exception handling or at least minimal handling like logging, plus a clear comment if intentionally ignoring the error. Here we want to preserve existing behavior: failures to query the bank or customer tables should not break the preview and should simply cause use of demo data, but the exception should be visible to maintainers.

Best fix without changing functionality:

• Keep the try / except structure and the existing default values (bank = None with a later fallback; customer = _demo_customer()).
• In both except Exception: blocks, add a log call using the existing logger (logging.getLogger(__name__) is already defined at line 27).
• Add a short comment documenting that the error is intentionally ignored for preview, while being logged.
• Use logger.exception(...) so the full traceback is captured, which is useful for debugging and does not change runtime control flow.

Concretely:

• In modules/invoice_designer/index.py, inside _preview_template:
  • Replace the except Exception:\n pass at lines 454–455 with an except Exception: that calls logger.exception(...) and adds a brief comment.
  • Similarly replace except Exception:\n pass at lines 470–471 with an except Exception: that logs the issue with a different message.

No new imports are necessary because logger is already defined; we only modify these two small blocks.

To fix a genuine “unused local variable” while preserving behavior, either remove the unnecessary assignment (if it truly has no side effects) or rename the variable to indicate it is intentionally unused. Here, the right-hand side is a constructor call (ParagraphStyle(...)), which is free of side effects in normal usage but might be relied upon in subtle ways (e.g., style registration). To avoid any risk, we should keep the assignment but rename the variable so static analysis understands it is intentionally unused.

The single best change here is to rename s_section on line 651 to something that clearly marks it as unused, for example unused_s_section. This satisfies the naming convention accepted by CodeQL (it contains unused) and does not change any functionality, since the name was not referenced elsewhere. No imports, methods, or additional definitions are needed—only the variable name in that line in modules/invoice_designer/index.py should be updated.

To fix an unused local variable, either delete it or rename it to clearly indicate that it is intentionally unused. Here, s_normal is one of many style variables defined together and appears to serve no purpose, so the best fix without changing functionality is to remove its definition entirely. This does not affect the behavior of the function, because no code reads s_normal.

Concretely, in modules/invoice_designer/index.py, within the function where the reportlab styles are defined (around lines 645–669), remove the line that assigns s_normal. No additional imports or definitions are needed, and no other references need updating because s_normal is not used anywhere.

In general, to fix an "unused local variable" warning, either remove the unused variable (if it truly is not needed) or, if it must remain for documentation or future use, rename it to follow an "unused" naming convention (e.g. _, _unused_meta_label, or unused_meta_label) so tools and readers understand it is intentionally unused.

Here, the simplest and least intrusive fix is to delete the unused s_meta_label definition on line 669 while leaving the right-hand side expression only if it has side effects. In this case, ParagraphStyle(...) is a pure constructor call with no side effects other than object creation, so we can safely remove the entire line without changing behavior. No other code is shown that depends on s_meta_label, and keeping the style for hypothetical future use is not necessary for correctness. Therefore, in modules/invoice_designer/index.py, in the function where styles are defined (around lines 645–672), remove the line that defines s_meta_label. No imports or additional definitions are needed.

In general, an unused local variable should either be removed (if it serves no purpose and its initializer has no side effects) or renamed to indicate that it is intentionally unused (e.g., _ or a name containing unused). Here, the ParagraphStyle construction has no side effects beyond creating an object, and s_meta_val is not used anywhere, so the best fix that preserves existing functionality is to remove this unused assignment entirely.

Concretely, in modules/invoice_designer/index.py, within the section where various ParagraphStyle instances are created, you should delete the line that defines s_meta_val (line 671 in the snippet). No additional imports, methods, or definitions are required. The surrounding style variables (s_meta_label, etc.) remain unchanged.

In general, to fix empty except blocks, either handle the error (e.g., log, clean up, apply fallback behavior) or narrow the exception type and document why it is safe to ignore. Here, we should preserve the current behavior of not raising errors to the caller, but avoid silently swallowing all information.

The best low-impact fix is to add logging inside the except block, using the existing logger defined at the top of the file. We keep return None as the outcome so behavior remains unchanged from the caller’s perspective. We also slightly narrow the except if possible; however, we do not know all exception types that storage.get, _io.BytesIO, or Image may raise, so keeping Exception is reasonable but we’ll log the exception object.

Concretely, in modules/invoice_designer/index.py, around lines 693–707, we will replace:

            try:
                result = storage.get(c['logo_path'])
                if result:
                    ...
                    return img
            except Exception:
                pass
            return None

with:

            try:
                result = storage.get(c['logo_path'])
                if result:
                    ...
                    return img
            except Exception as exc:
                logger.warning("Failed to load or render invoice logo from path %r: %s", c.get('logo_path'), exc)
            return None

This uses the already-imported logging module and logger instance, adds diagnostic information, and preserves the final return None behavior.