Skip to content

Add digital object type params and sf_format check - #2385

Open
anvit wants to merge 2 commits into
qa/2.xfrom
dev/fix-sf-params
Open

Add digital object type params and sf_format check#2385
anvit wants to merge 2 commits into
qa/2.xfrom
dev/fix-sf-params

Conversation

@anvit

@anvit anvit commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Add a check for digital object type params and sf_format and assume safe defaults in case on invalid values.

Add a check for digital object type params and sf_format and assume
safe defaults in case on invalid values.
@anvit anvit self-assigned this Jul 7, 2026
@anvit anvit added the Type: bug A flaw in the code that causes the software to produce an incorrect or unexpected result. label Jul 7, 2026
@anvit anvit added this to the 2.11 milestone Jul 7, 2026
@anvit
anvit requested a review from a team July 7, 2026 23:29

@sbreker sbreker 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.

Looks great @anvit! 👍

Comment on lines +137 to +142
$validMediaTypes = [
'audio' => QubitTerm::AUDIO_ID,
'image' => QubitTerm::IMAGE_ID,
'text' => QubitTerm::TEXT_ID,
'video' => QubitTerm::VIDEO_ID,
];

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.

Could we add 'other' => QubitTerm::OTHER_ID to this list? When a user selects "Other" the request sends mediatypes=139. This list does not include 139, so the code changes it to Image and shows the wrong results, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch! Defaulting to Other does make more sense than defaulting to text. Done!

Comment on lines +743 to +747
$validFormats = ['xml', 'ead', 'dc', 'csv', 'html', 'json'];

if (false === array_search($format, $validFormats)) {
$format = 'html';
}

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 blocking formats already registered in $this->formats. For example, txt is registered by Symfony, but this code changes it to html. Is this intentional? Maybe we can also accept the registered formats and use html only for values that are not known?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!

Update the default media type in case of invalid media types to be
'Other'. Also use the existing list for formats for sfWebRequest instead
of defining one just for the function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: bug A flaw in the code that causes the software to produce an incorrect or unexpected result.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants