Skip to content

fix: follow-up fixes for logo sync and filament spool_count#3

Merged
akira69 merged 18 commits intomasterfrom
feat/manufacturer-logo-labels
Feb 23, 2026
Merged

fix: follow-up fixes for logo sync and filament spool_count#3
akira69 merged 18 commits intomasterfrom
feat/manufacturer-logo-labels

Conversation

@akira69
Copy link
Copy Markdown
Owner

@akira69 akira69 commented Feb 21, 2026

Summary

Follow-up PR on top of merged fork work to harden logo sync behavior, spool-count handling, and UI consistency.

Fixes Included

  • Preserve existing vendor logos on sync failure by staging downloaded content and only swapping after successful validation/write.
  • Support arbitrary Git refs (branch/tag/SHA) for vendor logo tarball downloads instead of forcing refs/heads/<branch>.
  • Use getAPIURL() for vendor logo sync calls in vendor edit so base-path deployments work.
  • Fix filament list spool_count sorting/filtering 500s and optimize the spool_count query path.
  • Add migration index on spool.filament_id with column-based duplicate detection to avoid redundant FK indexes.
  • Co-extruded preview + table/filter UX follow-up fixes included from this branch iteration.

Validation

  • PYTHONPYCACHEPREFIX=/tmp/pycache python3 -m py_compile spoolman/vendor_logos.py
  • PYTHONPYCACHEPREFIX=/tmp/pycache python3 -m py_compile migrations/versions/2026_02_20_1605-b5f9c2e31a1b_add_index_on_spool_filament_id.py
  • docker run --rm -v "$PWD:/app" -w /app/client -e VITE_APIURL=/api/v1 -e NPM_CONFIG_CACHE=/tmp/.npm -e XDG_CONFIG_HOME=/tmp/.config -e HOME=/tmp node:20 /bin/bash -lc "npm ci && npm run build"

Manual Checks

  • Vendor edit: GitHub logo sync works under base-path deployments.
  • Vendor logo sync safety: failed/aborted sync does not delete existing runtime logos.
  • Filament/spool list: spool_count sorting/filtering works without 500s.
  • UI checks: clear-filters state, column controls, and color preview behavior.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 578f7daf6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@akira69
Copy link
Copy Markdown
Owner Author

akira69 commented Feb 21, 2026

Upstream References

This work builds on the filament label flow introduced in PR Donkie#846 and extends it with manufacturer-logo support and related UX improvements.

Addresses / Advances Existing Upstream Requests

Relation to Earlier PR

  • Follow-up to PR feat(printing): filament label printing foundation Donkie/Spoolman#846 (filament print/export + AML/PNG groundwork).
  • This PR adds manufacturer logo capabilities and UI/flow improvements on top of that baseline.
  • Any logo-sync robustness/base-path/ref handling fixes are fixes to this new feature set (not pre-existing upstream main bugs).

@akira69 akira69 force-pushed the feat/manufacturer-logo-labels branch 4 times, most recently from 51b49a3 to 0a7a235 Compare February 21, 2026 17:42
@akira69
Copy link
Copy Markdown
Owner Author

akira69 commented Feb 22, 2026

Follow-up QA note:

  • Updated table column double-click auto-fit logic so it measures intrinsic cell content and only expands when content is actually overflowing (prevents the prior "always grows a bit" behavior).
  • Rebuilt and redeployed to LXC for runtime validation; service health is OK after deploy.

I also cleaned up the PR description text so the fix bullets show the correct code references (for example, getAPIURL() and refs/heads/<branch> details).

@akira69 akira69 force-pushed the feat/manufacturer-logo-labels branch from 8b0be0f to bfa9681 Compare February 22, 2026 15:14
@akira69
Copy link
Copy Markdown
Owner Author

akira69 commented Feb 22, 2026

Branch cleanup complete:

  • Force-updated feat/manufacturer-logo-labels to the cleaned commit series from feat/manufacturer-logo-labels-clean.
  • Folded the latest auto-fit bugfix into the existing table commit (no extra standalone bugfix commit added).
  • PR now includes latest commit bfa9681 (feat(tables): pin Actions column and add double-click auto-fit for resized columns).

Also updated PR description with an explicit upstream-linkage section so it is clear which changes map to upstream requests and which are UX hardening follow-ups.

@akira69 akira69 force-pushed the feat/manufacturer-logo-labels branch from bfa9681 to 99ff9c2 Compare February 23, 2026 15:28
@akira69 akira69 merged commit dccb123 into master Feb 23, 2026
@akira69 akira69 deleted the feat/manufacturer-logo-labels branch February 23, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant