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

Features/250 dataset list integration #274

Merged
merged 20 commits into from
Jan 11, 2024

Conversation

ekraffmiller
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR closes:

  • Closes #

Special notes for your reviewer:

Suggestions on how to test this:

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

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

Additional documentation:

@coveralls
Copy link

coveralls commented Jan 7, 2024

Coverage Status

coverage: 97.922%. remained the same
when pulling f7b6f61 on features/250-dataset-list-integration
into b7fa0f5 on develop.

@ekraffmiller ekraffmiller marked this pull request as ready for review January 7, 2024 22:45
@ekraffmiller ekraffmiller added the pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows label Jan 7, 2024
@ekraffmiller ekraffmiller linked an issue Jan 7, 2024 that may be closed by this pull request
@MellyGray MellyGray self-assigned this Jan 8, 2024
@MellyGray MellyGray added integration Tasks involving the connection and interaction of UI features with the Dataverse API Size: 10 A percentage of a sprint. 7 hours. labels Jan 8, 2024
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.

The Home is working in localhost! 👏

I left a few comments about the code

src/dataset/infrastructure/mappers/JSDatasetMapper.ts Outdated Show resolved Hide resolved
src/dataset/domain/models/DatasetPreview.ts Outdated Show resolved Hide resolved
src/dataset/domain/models/DatasetPreview.ts Outdated Show resolved Hide resolved
src/dataset/domain/models/Dataset.ts Outdated Show resolved Hide resolved
src/dataset/domain/models/Dataset.ts Outdated Show resolved Hide resolved
tests/e2e-integration/e2e/sections/home/Home.spec.ts Outdated Show resolved Hide resolved
@MellyGray MellyGray removed their assignment Jan 10, 2024
@GPortas GPortas self-assigned this Jan 10, 2024
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.

Overall this looks very good! It is really nice to see the home page working with real data. Nice work @ekraffmiller @MellyGray !

I have done different tests, which I share below with videos and the steps to reproduce.

I'm using emojis (🚨 and ✅) to explain if the behavior is working as expected or if revision is required.

TEST 1 - Create first dataset from empty installation

creatingvideo.mov

Steps:

  1. Dataverse is empty, so I log in via JSF to create a data set.
  2. I create a test dataset through JSF
  3. I navigate to the JSF home page to see how the created dataset card is displayed in the list.
  4. ✅ I navigate to the SPA home page to verify that the new dataset is displayed in the list with the same appearance as in JSF.
  5. 🚨 I click the dataset in the list and navigate to the dataset page. I see that when navigating, instead of directly opening the dataset page, it goes through an intermediate page where a text "loading" appears (very quickly, but noticeable). See the following screenshot:
LOADINGPAGE
  1. ✅ The dataset page correctly displays the requested dataset

TEST 2 - Publish the dataset to verify that information is updated in the home page list

  1. I publish the dataset through JSF
  2. ✅ I refresh the home page in the SPA to verify the dataset info is updated
publishingvideo.mov

TEST 3 - Pagination: verify that pages are displayed correctly and pagination controllers work as expected

  1. I created more than 10 datasets via JSF in order to have two result pages
  2. ✅ In the SPA, each page is correctly rendered with its associated elements
  3. ✅ Pagination controllers work as expected
paginationtest1.mov

TEST 4 - Pagination: Back button and URL page query param

🚨

In JSF, a query parameter is added to the URL to indicate the results page you are on. I think we should continue to support this. It is useful for scenarios like the following:

  • When you have navigated to a new result page, by clicking on the browser's back button you navigate to the previously selected page.

  • With the query param you are also able to share a URL that leads directly to a particular page on the home page.

You can see in the following video how, by not adding the query parameter, the back button navigates to the page before reaching the home page.

paginationtest2.mov

@ekraffmiller
Copy link
Contributor Author

ekraffmiller commented Jan 10, 2024

As discussed at standup, I created new issues based on QA of this PR.

@ekraffmiller ekraffmiller removed their assignment Jan 10, 2024
@GPortas GPortas merged commit baa7faf into develop Jan 11, 2024
11 of 14 checks passed
@GPortas GPortas deleted the features/250-dataset-list-integration branch January 11, 2024 08:30
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…ration

Features/250 dataset list integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration Tasks involving the connection and interaction of UI features with the Dataverse API pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows Size: 10 A percentage of a sprint. 7 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display the list of datasets in the Home Page
4 participants