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 additional Entities to an Entity List by uploading a spreadsheet #589

Closed
matthew-white opened this issue Jan 17, 2024 · 15 comments · Fixed by getodk/central-frontend#961
Assignees
Labels
backend Requires a change to the API server behavior verified Behavior has been manually verified enhancement New feature or behavior entities Multiple Encounter workflows frontend Requires a change to the UI

Comments

@matthew-white
Copy link
Member

See the story in the release criteria.

@matthew-white matthew-white added enhancement New feature or behavior backend Requires a change to the API server frontend Requires a change to the UI needs testing Needs manual testing needs design review Needs verification from designer entities Multiple Encounter workflows labels Jan 17, 2024
@srujner
Copy link

srujner commented Mar 22, 2024

Hey, we found some strange issue probably with converting csv file.
I'm using OS: Ubuntu 20.04.5 LTS and Libre Office Calc for creating and saving csv file. If we will follow this steps:

  1. Navigate to Entities -> Entity name -> Data Tab -> Upload button;
  2. Click on the Download a data template (.csv)
  3. Fill the generated csv file and save it.
  4. Go back to Central, click on the choose one button and pick the file that has been just saved.

We will see this error: "There is a problem on row X of the CSV file: The label is missing."
Screenshot(48)
In the Text editor looks like there is no row number X and that the csv file should be ok.
Then, at a meeting with Kathleen, we exchanged files.
The file that @ktuite generated worked correctly and ours did not, but we were unable to find the cause.

Project URL:
https://test.getodk.cloud/#/projects/516/entity-lists/trees/entities

csv file:
trees.csv

@ktuite
Copy link
Member

ktuite commented Mar 22, 2024

Another request from the meeting was for a quick recap of what is functional and what is still missing in terms of CSV validation. More details in the release criteria but here's a summary of what is current:

  • header:

    • order of label and properties can be in any order
    • extra properties not allowed, missing properties not allowed
  • data:

    • label cannot be blank
      • label cannot be a string of spaces
    • every property value is a string
      • not sure what happens if you put something weird in a cell, like an image, and export it as a csv somehow?
        • matt tried uploading an image file instead of a csv and it showed some of the jpeg header data in the error message
    • there aren't a lot of other restrictions on what can be in the label or property data columns, it just has to be a valid CSV. adding quotes and extra commas can make the CSV invalid.
      • there are more restrictions for the name of the entity list or name of the property.
    • even though the column headers have to match the full list of properties, the values below don't have to all be filled in
    • because these CSVs don't include UUIDs, and the UUIDs are assigned on upload, duplicate rows/labels/other values are allowed
  • not finished yet (as of march 22):

    • eventually there will be warnings about things like ragged columns

@matthew-white
Copy link
Member Author

matthew-white commented Mar 26, 2024

Thanks, @ktuite!

In terms of status, everything described in the release criteria for the story should now be implemented. We haven't implemented warnings yet, but that's a separate story (#593).

not sure what happens if you put something weird in a cell, like an image, and export it as a csv somehow?

I can look into this. I think it'd be possible to construct a CSV file with a valid header but binary data in the rows below. I think it'd get rejected somewhere along the pipeline (by Postgres if not before), but I'll check.

@matthew-white
Copy link
Member Author

If we will follow this steps: ... We will see this error: "There is a problem on row X of the CSV file: The label is missing."

This should be fixed now on the QA server.

@srujner
Copy link

srujner commented Mar 27, 2024

Hey, we've got a little trouble with that dialog.
Both Dominica and I, without consulting each other beforehand, thought it was a simple 'tooltip information' and by clicking on the 'x' button we assumed that the box itself would disappear, but instead the entire list of uploaded entities was removed.
So we think it may not be intuitive enough for the user. Maybe we should make the dialog box bigger or the "x" should be in a different place?

Screenshot(51)

@matthew-white
Copy link
Member Author

CC @issa-tseng and @alyblenkin for input from the design team. I chatted about this briefly with @lognaturel, and one idea she had was to replace the x button with a trash can icon. I think that'd make the intention of the button clearer.

@alyblenkin
Copy link

@matthew-white Yes, I think the bin icon would be more helpful! It's a pattern we use already, so it's nice to be consistent. And when you hover over it, would the text say delete?

@matthew-white
Copy link
Member Author

A tooltip for the icon sounds great to me. Happy to run with "delete" or any other text.

@alyblenkin
Copy link

I noticed we use "Delete" in other places so I think that makes sense!
Screenshot 2024-03-27 at 2 17 56 PM

@matthew-white
Copy link
Member Author

I'm thinking I'll make the bin red as in the screenshot here, but I'm also happy to leave it gray.

@alyblenkin
Copy link

I think the red helps make it very clear what will happen.

@matthew-white
Copy link
Member Author

This is sort of a tangent, but I don't think the More button is supposed to wrap in your screenshot above. I've filed the issue #620 to fix that.

@srujner
Copy link

srujner commented Apr 23, 2024

Tested with Success!

1 similar comment
@dbemke
Copy link

dbemke commented Apr 23, 2024

Tested with Success!

@dbemke dbemke added behavior verified Behavior has been manually verified and removed needs testing Needs manual testing labels Apr 23, 2024
@matthew-white
Copy link
Member Author

not sure what happens if you put something weird in a cell, like an image, and export it as a csv somehow?

I can look into this. I think it'd be possible to construct a CSV file with a valid header but binary data in the rows below.

I took a look at this in getodk/central-frontend#982. See the PR description for details.

@matthew-white matthew-white removed the needs design review Needs verification from designer label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Requires a change to the API server behavior verified Behavior has been manually verified enhancement New feature or behavior entities Multiple Encounter workflows frontend Requires a change to the UI
Projects
Status: ✅ done
Development

Successfully merging a pull request may close this issue.

5 participants