Skip to content

Refactor CoverController to actions.#5367

Open
EreMaijala wants to merge 1 commit into
vufind-org:dev-12.0from
EreMaijala:dev-12.0-refactor-covercontroller
Open

Refactor CoverController to actions.#5367
EreMaijala wants to merge 1 commit into
vufind-org:dev-12.0from
EreMaijala:dev-12.0-refactor-covercontroller

Conversation

@EreMaijala

Copy link
Copy Markdown
Contributor

No description provided.

@EreMaijala EreMaijala requested a review from demiankatz June 10, 2026 13:35

@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, @EreMaijala -- see below for some minor questions/comments.

* @return ResponseInterface
*/
public function action(
ServerRequestInterface $request,

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.

Do we need an unused parameter annotation here since $request is not used? (I imagine this may be a widespread problem...)

// then on each page load. Default TTL set at 14 days
if ($this->config['Content']['coverimagesBrowserCache'] ?? true) {
$coverImageTtl = (60 * 60 * 24 * 14); // 14 days
$response = $response->withHeader(

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.

Maybe this is a follow-up action, but do we want to have a cache control helper so we can centralize and reuse this type of logic?

* @return ResponseInterface
*/
public function action(
ServerRequestInterface $request,

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.

$request also unused here.

@demiankatz demiankatz added this to the 12.0 milestone Jun 11, 2026
@demiankatz demiankatz added the architecture pull requests that involve significant refactoring / architectural changes label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

architecture pull requests that involve significant refactoring / architectural changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants