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

Ignore field names in line 1 of batch upload #74

Closed
philnorc opened this issue Apr 4, 2022 · 5 comments
Closed

Ignore field names in line 1 of batch upload #74

philnorc opened this issue Apr 4, 2022 · 5 comments
Assignees

Comments

@philnorc
Copy link

philnorc commented Apr 4, 2022

  1. From the Earnings table, export a record of status "Calculated"
  2. Open the download file with text editor and supply the missing values in line 2.
  3. Keep line 1, the field names, in place.
  4. Save the file.
  5. At the Payments table, Upload the file.
  6. The error message says:
    Payments Batch Upload Failed! "paid_at" must be in ISO 8601 date format

I see 2 problems with that:

  1. Uploading the field names is a common and easy mistake. We can easily fix our code to ignore it; to ignore line 1 and accept the data in line 2.

  2. "Paid_at" does not need to be in ISO 8601 format. That is not the date format we display in the tables. Try uploading data with a date like "April 4, 2022". The reply is "Data Uploaded Successfully."

FIX:
Let's change our code either of 2 ways:

  1. At the client, when line 1 has the field names, delete line 1 before posting the data.
  2. At the server, ignore line 1 when accepting data and writing SQL.
@dadiorchen
Copy link
Contributor

@philnorc we suppose the user should use app like excel or numbers (macos) to deal with the file, so in this way, we should keep the first line as the header of the table.
For the ISO format, I didn't read the detailed specification for the standard, but I think it's fine to let people input several different formats, we are using this API to do the data verification: https://joi.dev/api/?v=17.6.0#date

@dadiorchen
Copy link
Contributor

And by using the excel, the app will help to fix problem and promise a good format of the data, in this issue, is about an edge case of using semicolon to separate fields, we are going to support this case, but currently, semicolon leads to a problem. Greenstand/treetracker-admin-api#631

@philnorc
Copy link
Author

philnorc commented Apr 5, 2022

ISO date format is YYYY-MM-DD.

I think it's a good idea. We should use it.
It makes it much easier to sort data chronologically.
An alphabetic sort, or a numeric sort, results in a chronological sort.

Maybe our tables sort OK and we don't care.
But we would save our stakeholders the trouble of finding special code to sort our data when they put it in their spreadsheets.

@dadiorchen
Copy link
Contributor

@philnorc next time let's just focus on one single problem in an issue, this makes us easier to manage the issues

@dadiorchen
Copy link
Contributor

@philnorc I think I have confirmed No.1 and open issue #75 as a bug

For no2, I don't think we should fix it, and also, I think I can unify the date format to YYYY-MM-DD, so both the tables and CVS will be consistent

Now I will close this issue

@dadiorchen dadiorchen moved this from Ready for test to Done in Earnings System Production Readiness Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

2 participants