Skip to content

Comments

Don't return file when some derived info is requested.#6244

Merged
kcondon merged 3 commits intoIQSS:developfrom
QualitativeDataRepository:IQSS/6240
Oct 4, 2019
Merged

Don't return file when some derived info is requested.#6244
kcondon merged 3 commits intoIQSS:developfrom
QualitativeDataRepository:IQSS/6240

Conversation

@qqmyers
Copy link
Member

@qqmyers qqmyers commented Oct 2, 2019

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Note that we use the "closes" syntax below to trigger Github's automation to close the corresponding issue once the pull request is merged.

Thanks for your contribution to Dataverse!

Related Issues

Pull Request Checklist

@dataversebot
Copy link

Can one of the admins verify this patch?

@coveralls
Copy link

coveralls commented Oct 2, 2019

Coverage Status

Coverage decreased (-0.003%) to 19.472% when pulling 9e14d7d on QualitativeDataRepository:IQSS/6240 into 064f152 on IQSS:develop.

qqmyers added a commit to QualitativeDataRepository/dataverse that referenced this pull request Oct 2, 2019
Comment on lines +316 to +317
boolean serviceRequested = false;
boolean serviceFound = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

The main change is to run through the loop of query params and to set serviceRequested = true if any of the ones required by a service are sent, and to then run the checkIfServiceSupportedAndSetConverter ~as before (now only run on service params) to decide if the specific service needed exists.

After the loop, a check to see if a service was requested but not found, and, if so, to throw a not found exception. Otherwise, we're good to go (as before)

if (key.equals("imageThumb") || key.equals("format") || key.equals("variables")) {
serviceRequested = true;
//Only need to check if this key is associated with a service
if (downloadInstance.checkIfServiceSupportedAndSetConverter(key, value)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

because this used to run for any params like :persistentId, gbrecs, key it wasn't possible to just put an exception in an else clause (as was commented out in the previous code.

Copy link
Contributor

@landreev landreev left a comment

Choose a reason for hiding this comment

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

Hi Jim,
Could you please sync the branch up with develop (mainly to increment the current version in pom.xml to 4.17, to make the QA process easier).
Looks good otherwise, thanks.

@landreev landreev removed their assignment Oct 4, 2019
@kcondon kcondon merged commit 5e97b88 into IQSS:develop Oct 4, 2019
@kcondon kcondon self-assigned this Oct 4, 2019
@djbrooke djbrooke added this to the 4.18 milestone Oct 7, 2019
@qqmyers qqmyers deleted the IQSS/6240 branch May 17, 2024 18:40
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.

Thumbnail api call returns original file when thumbnail not available

6 participants