Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add use cases to get the zip download limit and embargo duration in months #89

Merged
merged 8 commits into from
Oct 13, 2023

Conversation

GPortas
Copy link
Contributor

@GPortas GPortas commented Sep 7, 2023

What this PR does / why we need it:

We need these new cases since they are necessary for the SPA to show the related functionality.

Which issue(s) this PR closes:

Related Dataverse PRs:

Special notes for your reviewer:

Tests related to files are failing, since the Dataverse branch they are pointing to does not have the files API extension. Therefore, this PR must wait for the PR IQSS/dataverse#9853 to be merged and IQSS/dataverse#9881 to be updated with develop.

Suggestions on how to test this:

  • Visual inspection of the code
  • Check unit and integration tests execution in GitHub actions

Is there a release notes update needed for this change?:

Add GetZipDownloadLimit and GetMaxEmbargoDurationInMonths use cases

Additional documentation:

None

@GPortas GPortas added the Waiting label Sep 7, 2023
@GPortas GPortas marked this pull request as ready for review September 7, 2023 09:42
@GPortas GPortas changed the title Add use cases to get the zip download limit and know if embargo functionality is enabled or not Add use cases to get the zip download limit and embargo duration in months Sep 11, 2023
Base automatically changed from 83-file-counts-use-case to develop September 11, 2023 18:03
@cmbz cmbz added the Size: 3 A percentage of a sprint. 2.1 hours. label Sep 13, 2023
@MellyGray MellyGray self-assigned this Oct 2, 2023
Copy link
Contributor

@MellyGray MellyGray left a comment

Choose a reason for hiding this comment

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

Looks good. Just a comment about splitting a test

Hopefully the dataverse branches this PR depends on get merged soon

test/integration/info/DataverseInfoRepository.test.ts Outdated Show resolved Hide resolved
@GPortas
Copy link
Contributor Author

GPortas commented Oct 3, 2023

@MellyGray

There are use cases whose integration tests are failing, that is normal, since the Dataverse branch used in this PR does not include all endpoints due to Dataverse PRs not yet merged.

@GPortas GPortas requested a review from MellyGray October 3, 2023 10:37
@GPortas GPortas removed their assignment Oct 3, 2023
@GPortas GPortas removed the Waiting label Oct 3, 2023
@MellyGray
Copy link
Contributor

MellyGray commented Oct 3, 2023

There are use cases whose integration tests are failing, that is normal, since the Dataverse branch used in this PR does not include all endpoints due to Dataverse PRs not yet merged.

@GPortas I know, but should the integration tests point to DATAVERSE_IMAGE_TAG=9880-info-zip-download-limit-embargo ? branch 9880 is already merged into develop so maybe you can point to the develop image tag so when the other files PR get merged into develop the integration tests work

I'm ready to approve this, just wanted to comment that

@GPortas
Copy link
Contributor Author

GPortas commented Oct 3, 2023

@MellyGray

I agree. In fact, in an ideal scenario the develop branch of this repository should always point to dataverse develop branch image (unstable tag).

Updating.

@GPortas GPortas removed their assignment Oct 3, 2023
Copy link
Contributor

@MellyGray MellyGray left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MellyGray MellyGray removed their assignment Oct 3, 2023
@kcondon kcondon self-assigned this Oct 13, 2023
@kcondon kcondon merged commit 4bac9cb into develop Oct 13, 2023
2 checks passed
@kcondon kcondon deleted the 84-read-settings branch October 13, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add use case to read settings required by the SPA (Embargo enabled and zip download limit)
4 participants