Devrel 1074 import taxonomies #9
Conversation
JiriLojda
force-pushed
the
DEVREL-1074_import_taxonomies
branch
from
d7356ba
to
306692b
Compare
|
|
||
| return { | ||
| ...context, | ||
| taxonomyGroupIdsByOldIds: new Map(zip(fileTaxonomies.map(t => t.id), projectTaxonomies.map(t => t.id))), |
There was a problem hiding this comment.
so we can assume that the order of taxonomies in the project are the same that are in the file? Even though that's probably true and makes sense, is there no possibility that the MAPI response can't mess this up and change the order?
There was a problem hiding this comment.
The order of taxonomies is intentional, it is not a dictionary, but a list. You can even reorder them in the UI (not sure about MAPI). So I would be really surprised if it reordered them from what I sent it.
| return { | ||
| ...context, | ||
| taxonomyGroupIdsByOldIds: new Map(zip(fileTaxonomies.map(t => t.id), projectTaxonomies.map(t => t.id))), | ||
| taxonomyTermIdsByOldIds: new Map(zip(fileTaxonomies.flatMap(t => t.terms), projectTaxonomies.flatMap(t => t.terms)).flatMap(extractTermIdsEntries)), |
There was a problem hiding this comment.
why do we need two Maps for taxonomies? we can have the same internal ID for taxonomy group and term? I think that it is not probable.. if not, what is the advantage having map for groups and terms?
There was a problem hiding this comment.
I thought it would be a better separation. You never have an id that can be a group or a term. In content types you reference groups in variants you reference terms so why not have them in two maps.
| arr1 | ||
| .slice(0, Math.min(arr1.length, arr2.length)) | ||
| .map((el1, i) => [el1, arr2[i]] as const) as unknown as Zip<T1, T2>; | ||
|
|
There was a problem hiding this comment.
this is really hard to follow.. maybe adding some comments, why is the array different from Tuple,
There was a problem hiding this comment.
You mean on the type-level or where?
JiriLojda
force-pushed
the
DEVREL-1074_import_taxonomies
branch
from
306692b
to
56ded2d
Compare
JiriLojda
force-pushed
the
DEVREL-1074_import_taxonomies
branch
from
56ded2d
to
08e2cd0
Compare
Motivation
Which issue does this fix? Fixes #
issue numberIf no issue exists, what is the fix or new feature? Were there any reasons to fix/implement things that are not obvious?
Checklist
How to test
If manual testing is required, what are the steps?