fix(migration): import TickTick data by column name instead of index #1356

kooshi · 2023-01-06T02:43:52Z

kooshi commented

2023-01-06 02:43:52 +00:00

A new column "Kind" was inserted to index 3 in the csv.

Also, handling of parent tasks was incorrect (parent id and task id were both reading from index 21)

A new column "Kind" was inserted to index 3 in the csv. Also, handling of parent tasks was incorrect (parent id and task id were both reading from index 21)

kooshi added 1 commit 2023-01-06 02:43:56 +00:00

continuous-integration/drone/pr Build is passing Details

af9faad25a fix ticktick migration, still need to update test to match

konrad reviewed 2023-01-06 09:38:12 +00:00

pkg/modules/migration/ticktick/ticktick.go Outdated

						
				@ -213,2 +212,3 @@

						parentID, err := strconv.ParseInt(record[23], 10, 64)

						if err != nil {

							return err

							parentID = 0

konrad commented

2023-01-06 09:38:12 +00:00

Isn't the parentID already 0 if there's an error?

If we're ignoring the error anyway we might as well just remove the if and do something like parentID, _ := strconv.ParseInt(record[23], 10, 64).

Isn't the `parentID` already `0` if there's an error? If we're ignoring the error anyway we might as well just remove the if and do something like `parentID, _ := strconv.ParseInt(record[23], 10, 64)`.

konrad commented

2023-01-06 09:39:11 +00:00

I think the tests only check if the conversion from TickTick's structure to Vikunja's works, not if the csv is correctly parsed. Do you want to add tests for that?

konrad commented

2023-01-06 09:41:20 +00:00

It's probably a good idea to switch the csv parsing to something like https://github.com/gocarina/gocsv so that we can avoid issues like this where they shuffle columns or add new ones in between in the future. (not in this PR though)

konrad commented

2023-01-11 21:00:14 +00:00

Any update here?

konrad commented

2023-01-22 12:02:15 +00:00

Resolves this issue: https://github.com/go-vikunja/api/issues/61

konrad added 1 commit 2023-01-23 18:13:22 +00:00

continuous-integration/drone/pr Build was killed Details

b96b10dc70

fix(migration): bullet-proof csv parsing for TickTick import

konrad added 1 commit 2023-01-23 18:15:04 +00:00

continuous-integration/drone/pr Build is failing Details

37240caad4

fix: lint

konrad changed title from ~~fix ticktick migration, still need to update test to match~~ to fix(migration): import TickTick data by column name instead of index

2023-01-23 18:15:50 +00:00

konrad commented

2023-01-23 18:17:23 +00:00

Now updated to pull the data by name from the csv file instead of relying on column indexes.

konrad scheduled this pull request to auto merge when all checks succeed 2023-01-23 18:17:58 +00:00

konrad commented

2023-01-23 18:18:19 +00:00

Thanks for your effort!

konrad added 1 commit 2023-01-24 13:22:11 +00:00