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

316 - Functional create dataset form #334

Merged
merged 18 commits into from
Mar 28, 2024

Conversation

MellyGray
Copy link
Contributor

@MellyGray MellyGray commented Mar 8, 2024

Here's a revised version of the text with corrected English:

What This PR Does / Why We Need It:

This PR introduces a functional 'Create Dataset' form, allowing users to create new datasets via the form. However, this version of the form has several limitations:

  1. It only includes the required fields.
  2. The metadata fields are not dynamically rendered; the UI is not connected to the backend for the configuration of these fields (TBD in Dynamically render Create Dataset Form fields based on the backend configuration #336 )
  3. In the JSF version, required fields could be multiple (e.g., there could be several author names). However, in this first iteration of the SPA, users can only add one author name. (TBD in Convert fields from the Create Dataset Form to multiple values field type #337)
  4. The validation is hardcoded, meaning that the SPA is not connected to the backend for metadata field validation. (TBD in Add validation library to validate fields from the Create Dataset Form #338)

This constitutes the first iteration of the 'Create Dataset' form. We plan to address these limitations in future updates.

Which Issue(s) This PR Closes:

Special Notes for Your Reviewer:

I made a few changes to the Design System form fields.

Below are the issues created to continue with the development of this feature:

Suggestions on How to Test This:

Step 1: Run the Development Environment

  1. Execute npm i.
  2. Navigate with cd packages/design-system && npm run build.
  3. Return with cd ../../.
  4. Ensure you have a .env file similar to .env.example, with the variable VITE_DATAVERSE_BACKEND_URL=http://localhost:8000.
  5. Navigate with cd dev-env.
  6. Start the environment using ./run-env.sh unstable.
  7. To verify the environment, visit http://localhost:8000 and check your local Dataverse installation.

Step 2: Test the Form

  1. Navigate to http://localhost:8000.
  2. Log in as an admin using the username: dataverseAdmin and password: admin1.
  3. Go to http://localhost:8000/spa and click the 'Add Data >> Add Dataset' button.
  4. Experiment with the form validation.
  5. Complete the form and click 'Save Dataset'; you should be redirected to your new dataset.

Does This PR Introduce a User Interface Change? If Mockups Are Available, Please Link/Include Them Here:

image

Is There a Release Notes Update Needed for This Change?:

Added the 'Create dataset' feature to the SPA.

Additional Documentation:

@coveralls
Copy link

coveralls commented Mar 8, 2024

Coverage Status

coverage: 97.539% (+0.3%) from 97.26%
when pulling 5b2343f on feature/316-create-functional-create-dataset-form
into 1ce4113 on develop.

@MellyGray MellyGray added Size: 3 A percentage of a sprint. 2.1 hours. pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows integration Tasks involving the connection and interaction of UI features with the Dataverse API labels Mar 8, 2024
@MellyGray MellyGray marked this pull request as ready for review March 11, 2024 14:50
@ekraffmiller ekraffmiller self-assigned this Mar 11, 2024
Copy link
Contributor

@ekraffmiller ekraffmiller left a comment

Choose a reason for hiding this comment

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

This looks really good!
I just have questions about the tests.
In FormGroupWithMultiple fields, the data entered disappears when adding and removing field groups. Is this just a behavior of the unit test, or is the data being lost?

Screen.Recording.2024-03-14.at.8.07.00.AM.mov

Also, when I run Form.spec.tsx, it seems to enter a infinite loop

Screen.Recording.2024-03-14.at.8.10.06.AM.mov

…into feature/316-create-functional-create-dataset-form
@MellyGray
Copy link
Contributor Author

@ekraffmiller

  1. I fixed the merge conflicts
  2. I fixed the Form.spec.tsx infinite loop
  3. The FormGroupWithMultiple never worked as expected. It's the problem with developing a design system component without actually using it in the source code. I was using React.cloneElement() when the user clicks the Add button, but it's really difficult to manage the props of the cloned elements, they disappear, you change the props and the element is not updated, etc. And I didn't realise this until I tried to use it in a real Form. I think we need to completely remove this element from the design system and work on the logic of these multiple fields in Convert fields from the Create Dataset Form to multiple values field type #337. If we find a way of adding it to the design system, great. But maybe it's easier to just have the different components separately and work on the logic of the Add Remove in the real Form.

@MellyGray MellyGray assigned ekraffmiller and unassigned MellyGray Mar 15, 2024
@ekraffmiller
Copy link
Contributor

@ekraffmiller

  1. I fixed the merge conflicts
  2. I fixed the Form.spec.tsx infinite loop
  3. The FormGroupWithMultiple never worked as expected. It's the problem with developing a design system component without actually using it in the source code. I was using React.cloneElement() when the user clicks the Add button, but it's really difficult to manage the props of the cloned elements, they disappear, you change the props and the element is not updated, etc. And I didn't realise this until I tried to use it in a real Form. I think we need to completely remove this element from the design system and work on the logic of these multiple fields in Convert fields from the Create Dataset Form to multiple values field type #337. If we find a way of adding it to the design system, great. But maybe it's easier to just have the different components separately and work on the logic of the Add Remove in the real Form.

Ok, that makes sense, once we get them working in the form, maybe a way to make it generic will be more doable. I think it's a great start on issue #337 , so I'd like to use it as a starting point. If we keep it there for now, then I can use it while working on that issue, and remove it as part of my PR. What do you think?

@MellyGray
Copy link
Contributor Author

Ok, that makes sense, once we get them working in the form, maybe a way to make it generic will be more doable. I think it's a great start on issue #337 , so I'd like to use it as a starting point. If we keep it there for now, then I can use it while working on that issue, and remove it as part of my PR. What do you think?

Sounds good, @ekraffmiller! Do you want me to make any other changes to this PR, or are we good to go?

@MellyGray MellyGray assigned ekraffmiller and unassigned MellyGray Mar 18, 2024
@ekraffmiller
Copy link
Contributor

good to go, approved!

ekraffmiller
ekraffmiller previously approved these changes Mar 18, 2024
Copy link
Contributor

@ekraffmiller ekraffmiller 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 to me!

@GPortas
Copy link
Contributor

GPortas commented Mar 25, 2024

@MellyGray Can you please resolve the merge conflicts?

@GPortas GPortas assigned MellyGray and unassigned GPortas Mar 25, 2024
…into feature/316-create-functional-create-dataset-form
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.

This works great and it is a very good starting point for the SPA version of the create dataset form.

createdataset.mov

@GPortas GPortas merged commit 75fe4fb into develop Mar 28, 2024
14 checks passed
@GPortas GPortas deleted the feature/316-create-functional-create-dataset-form branch March 28, 2024 10:42
@GPortas GPortas removed their assignment Mar 28, 2024
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…reate-dataset-form

316 - Functional create dataset form
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 pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 3 A percentage of a sprint. 2.1 hours. SPA: Create Dataset Form
Projects
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Create functional create dataset form with "hardcoded" citation metadata fields
4 participants