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

134 - Files table UI [1/2] - Integration display data #169

Merged
merged 41 commits into from
Sep 22, 2023

Conversation

MellyGray
Copy link
Contributor

@MellyGray MellyGray commented Aug 30, 2023

What this PR does / why we need it:

This PR adds the integration of the js-dataverse use cases to get the files into the SPA UI

Which issue(s) this PR closes:

Special notes for your reviewer:

Changes

  1. js-dataverse getDatasetFiles added to the FileRepository js-dataverse implementation
  2. js-dataverse getFileUserPermissions added to the FileRepository js-dataverse implementation
  3. js-dataverse getFileDownloadCount added to the FileRepository js-dataverse implementation
  4. Multiple fixes in the Files Tab logic derived from the integration
  5. Integration tests for the FileJSDataverseRepository added
  6. e2e tests to check the correctly display of the Files Tab added

Notes

The total files count is not yet integrated into the js-dataverse module so the number is still mocked

Also, the e2e tests are failing in the Github Action because the dataverse branch haven't been merged yet. But they should work locally npm run test:e2e

Suggestions on how to test this:

Step 1: Run the development environment

  1. Run npm I
  2. cd packages/design-system && npm run build
  3. cd ../../
  4. Check that you have a .env file such as the .env.example, with the VITE_DATAVERSE_BACKEND_URL=http://localhost:8000 variable
  5. cd dev-env
  6. ./run-env.sh 9692-files-api-extension-display-data
  7. To check the environment go to http://localhost:8000 and check your local dataverse installation

Step 2: Test Dataset View mode with the implemented changes for the getDatasetFiles

  1. Go to http://localhost:8000
  2. Login as admin using username: dataverseAdmin and password: admin1
  3. Create a new Dataset
  4. Upload some files
  5. From the dataset view mode copy the search parameters from the url. Ex.: ?persistentId=doi:10.5072/FK2/LHGRHP&version=DRAFT
  6. Go to http://localhost:8000/spa/datasets and paste the search parameters. Ex.: http://localhost:8000/spa/datasets?persistentId=doi:10.5072/FK2/LHGRHP&version=DRAFT
  7. You are seeing now the same Dataset in the SPA, check that all the data is displayed correctly regarding the Files Tab. Note that some api calls are still mocked such as the number of files and the filters, only the display of the files data and the Sort button should work.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No

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

Files Tab data display integrated with the backend

Additional documentation:

@MellyGray MellyGray changed the title Feature/134 integration files table display data 134 - Files table UI [1/2] - Integration display data Aug 30, 2023
@MellyGray MellyGray added the Size: 3 A percentage of a sprint. 2.1 hours. label Aug 30, 2023
@MellyGray
Copy link
Contributor Author

I have left some comments in the code and below I share some functionality details to fix.

SPA: spa_notes

Thanks for your detailed review! I applied most of the changes, just one thing to discuss:

Regarding point 1 of your screenshot. I see 2 options to fix it:

  1. In JSF this property is read directly from dataFile as fileMetadata.dataFile.friendlyType, should js-dataverse module retrieve this friendlyType property?
  2. If we think this is a frontend responsibility then I can upload the mapping file from JSF to the SPA to perform this mapping directly in the SPA. But then the rest of frontends using the API won't have access to this mapping.

Let me know which approach do you prefer

@MellyGray MellyGray assigned GPortas and unassigned MellyGray Sep 19, 2023
@GPortas
Copy link
Contributor

GPortas commented Sep 20, 2023

@MellyGray

Let's add the friendlyType property, both to the js-dataverse model and to the associated file API payload. Since, as you say, it may be useful for other applications and is also straightforward to add. You can keep it as it is for now, make note of this change and implement it once we have implemented it in js-dataverse/API.

I have run the application again to test the latest changes. I have only found a small detail to correct.

JSF:
jsf_md5

SPA:
spa_md5

You can see how there is a space missing between MD5: and the value in the SPA version.

MD5:f56...22b

But it should be like

MD5: f56...22b

@GPortas
Copy link
Contributor

GPortas commented Sep 20, 2023

There are some conflicts in this PR to be resolved.

@MellyGray
Copy link
Contributor Author

@GPortas I fixed the MD5: f56...22b and the merge conflicts!

@MellyGray MellyGray removed their assignment Sep 20, 2023
Copy link
Contributor

@GPortas GPortas left a comment

Choose a reason for hiding this comment

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

LGTM!

@GPortas GPortas removed their assignment Sep 21, 2023
Base automatically changed from feature/159-manage-user-permissions-files to develop September 22, 2023 07:37
@kcondon kcondon self-assigned this Sep 22, 2023
@kcondon
Copy link
Contributor

kcondon commented Sep 22, 2023

@GPortas @MellyGray I think embargo is not enabled in the container so can't configure and check but looks good, merging.

@kcondon kcondon merged commit a220c4f into develop Sep 22, 2023
5 of 6 checks passed
@kcondon kcondon deleted the feature/134-integration-files-table-display-data branch September 22, 2023 15:26
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…able-display-data

134 - Files table UI [1/2] - Integration display data
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.

[Spike - Integration] [1/2] Create the File Tab of the Dataset page - Display data
4 participants