Skip to content

Conversation

@wjarka
Copy link

@wjarka wjarka commented Nov 28, 2025

redesign Lookup Provider architecture for extensibility and type safety type safety

This refactor modernizes the Barcode Lookup Provider system to improve extensibility, type safety, and maintainability:

Core Architecture Changes:

  • Made LookupProvider abstract with required methods (getName, getDescription, getConfigKey) to enforce compile-time contracts for all providers
  • Replaced hardcoded provider list with dynamic provider registry in config-dist.php (LOOKUP_PROVIDERS constant) for easier addition of new providers
  • Implemented getProviders() factory method in BarcodeLookup to dynamically load providers from configuration

Provider Interface Improvements:

  • Added getConfigHtml() method for providers to define their own configuration UI
  • Added getConfigFieldId() to identify main config field for dynamic enable/disable
  • Added saveSettings() hook for custom provider save logic
  • Added setId()/getId() for unique provider identification
  • Unified enablement check to single isEnabled() method

Settings UI Enhancements:

  • Moved provider-specific configuration fields from settings.php into respective provider classes (better separation of concerns)
  • Implemented generic JavaScript handler for toggling config fields based on provider enablement checkbox
  • Improved UI layout with proper vertical stacking of provider description and config fields

Code Quality:

  • Removed all require_once statements for individual providers from LookupProvider base class (now loaded dynamically)
  • Added explicit type casting for EditFieldBuilder output to satisfy Psalm
  • Updated Psalm baseline to reflect resolved type issues
  • Improved error messages to use getName() instead of property access

This refactor maintains backward compatibility while making it significantly easier to add new lookup providers - developers now only need to:

  1. Create a new provider class extending LookupProvider
  2. Implement required abstract methods
  3. Add entry to LOOKUP_PROVIDERS in config-dist.php

… type safety

This refactor modernizes the Barcode Lookup Provider system to improve
extensibility, type safety, and maintainability:

Core Architecture Changes:
- Made LookupProvider abstract with required methods (getName, getDescription,
  getConfigKey) to enforce compile-time contracts for all providers
- Replaced hardcoded provider list with dynamic provider registry in config-dist.php
  (LOOKUP_PROVIDERS constant) for easier addition of new providers
- Implemented getProviders() factory method in BarcodeLookup to dynamically load
  providers from configuration

Provider Interface Improvements:
- Added getConfigHtml() method for providers to define their own configuration UI
- Added getConfigFieldId() to identify main config field for dynamic enable/disable
- Added saveSettings() hook for custom provider save logic
- Added setId()/getId() for unique provider identification
- Unified enablement check to single isEnabled() method

Settings UI Enhancements:
- Moved provider-specific configuration fields from settings.php into respective
  provider classes (better separation of concerns)
- Implemented generic JavaScript handler for toggling config fields based on
  provider enablement checkbox
- Improved UI layout with proper vertical stacking of provider description and
  config fields

Code Quality:
- Removed all require_once statements for individual providers from LookupProvider
  base class (now loaded dynamically)
- Added explicit type casting for EditFieldBuilder output to satisfy Psalm
- Updated Psalm baseline to reflect resolved type issues
- Improved error messages to use getName() instead of property access

This refactor maintains backward compatibility while making it significantly
easier to add new lookup providers - developers now only need to:
1. Create a new provider class extending LookupProvider
2. Implement required abstract methods
3. Add entry to LOOKUP_PROVIDERS in config-dist.php
@wjarka
Copy link
Author

wjarka commented Nov 28, 2025

Hey @Forceu, don't mean to be pushy - just not sure how often you check this section (as there a few PRs sitting there for a while). Would appreciate some feedback if needed and potentially merging it as I would like to add some providers as next steps :)

@wjarka
Copy link
Author

wjarka commented Nov 28, 2025

Ahh, a quick note - upgrade will require people updating their data/config.php files to move the providers configuration (I kept the IDs the same for backwards compatibility, so the order and enabled / disabled should stay intact).

@Forceu
Copy link
Owner

Forceu commented Nov 28, 2025

Thanks a lot for your PR! At the moment I am unfortunately very busy and don't have as much time for this project as I would like to :/ I will however review this PR as soon as possible

@wjarka
Copy link
Author

wjarka commented Nov 28, 2025

Thanks a lot for your PR! At the moment I am unfortunately very busy and don't have as much time for this project as I would like to :/ I will however review this PR as soon as possible

No worries, completely understandable. As I am setting myself up with grocy and barcodebuddy - I will be working on some features, so let me know if there is any way I can help :)

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.

2 participants