Skip to content

Use config array instead of object for Email and Secret trait features - #5656

Draft
rtloftus wants to merge 3 commits into
vufind-org:devfrom
rtloftus:trait
Draft

Use config array instead of object for Email and Secret trait features#5656
rtloftus wants to merge 3 commits into
vufind-org:devfrom
rtloftus:trait

Conversation

@rtloftus

@rtloftus rtloftus commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

relies on eds backend test which is in PR #5654 as of right now. Waiting for blender to be merged

@demiankatz demiankatz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, @rtloftus! When changing traits, don't forget to check all the references to those traits. Both of these still have a few calls that need updating -- see details below.

* @return string
*/
protected function getEmailSenderAddress(array|Config $config, ?string $userEmail = null): string
protected function getEmailSenderAddress(array $config, ?string $userEmail = null): string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are calls to this method in \VuFind\Log\LoggerFactory::addMailHandler and \VuFindConsole\Command\ScheduledSearch\NotifyCommand that still pass a Config object. We'll need to update them before we can change the signature of this method.

* @return string|null
*/
protected function getSecretFromConfig(Config|array|null $config, string $key): ?string
protected function getSecretFromConfig(array|null $config, string $key): ?string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is still being called with an object in \VuFind\Search\Factory\BrowZineBackendFactory and \VuFindSearch\Backend\EDS\Backend. Those calls will need to be updated to pass the right format -- this is the reason tests are failing.

@rtloftus
rtloftus marked this pull request as draft September 10, 2026 18:36
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