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

Support reference resolution in the key field not only id in ResourceIdentifiers #138

Closed
heshamMassoud opened this issue Oct 10, 2017 · 4 comments

Comments

@heshamMassoud
Copy link
Contributor

heshamMassoud commented Oct 10, 2017

Edit: The task is resolved, so only parts are related to the API, not SDK anymore, when API adds support of ResourceIdentifier for PriceDraft#customerGroup we could close the issue. (Currently it's not clear when API will support this.)

Remaining work:

Support requests (SUPPORT-9797, SUPPORT-9798) are opened for the problematic types/fields both API and SDK. Additionally, a GitHub issue is opened for jvm-sdk types: commercetools/commercetools-jvm-sdk#2052 (Edit: The task is resolved, so only parts are related to the API, not SDK anymore.)


Description

Right now there are references which are of type Reference and other references which are of type ResourceIdentifier

  1. Reference only supports id as a field.
  2. ResourceIdentifier supports id or key as a field.

On CTP in the future, all references should be changed to ResourceIdentifier.

It's quite confusing for the user of the sync library to put the key in the id field for reference resolution.

Current Behaviour

Whether it's a resourceIdentifier or reference reference resolution is always done on the id field.

Expected Behaviour

  1. When the reference is of type ResourceIdentifier:
    • If the key field is set, sync lib should check if resource exists or not, and keep track of missing references (as done in cat sync) as it might be resolved in later batches.
    • If the id field is set, then it is an evidence of resource existence on CTP, so we can issue an update/create API request right away.
  2. When the reference is of type Reference:
    • Do reference resolution as it currently is (key -> id mapping).

Related: #231

@heshamMassoud heshamMassoud added this to the backlog milestone Oct 10, 2017
@heshamMassoud heshamMassoud modified the milestones: backlog, Stabilisation Milestone (Previously: 1.0.0-M2-beta), 1.0.0-M11 (Reference Resolution Milestone) Jan 19, 2018
@heshamMassoud heshamMassoud modified the milestones: 1.0.0-M12 (Reference Resolution Milestone), 1.0.0-M10, 1.0.0-M11 Feb 9, 2018
@heshamMassoud heshamMassoud modified the milestones: 1.0.0-M11, 1.0.0-M12 Mar 6, 2018
@heshamMassoud heshamMassoud self-assigned this Mar 9, 2018
@heshamMassoud heshamMassoud modified the milestones: 1.0.0-M13, 1.0.0-M14 Sep 5, 2018
@heshamMassoud heshamMassoud modified the milestones: 1.0.0-M14, 1.0.0-M15 Oct 4, 2018
heshamMassoud added a commit that referenced this issue Oct 9, 2018
heshamMassoud added a commit that referenced this issue Oct 14, 2018
heshamMassoud added a commit that referenced this issue Oct 14, 2018
@butenkor
Copy link
Member

butenkor commented Aug 26, 2020

As defined by @ahmetoz

References on commercetools-sync-java

ProductDraft

field commercetools type required/optional JVM SDK draft type status
productType ResourceIdentifier to a ProductType Required ResourceIdentifier<ProductType> ✔️
categories Array of ResourceIdentifier for a Category Optional Set<ResourceIdentifier<Category>> ✔️
taxCategory ResourceIdentifier to a TaxCategory Optional ResourceIdentifier<TaxCategory> ✔️
state ResourceIdentifier to a State Optional ResourceIdentifier<State> ✔️

PriceDraft

field commercetools type required/optional JVM SDK draft type status
customerGroup Reference to a CustomerGroup Optional Reference<CustomerGroup> ⚠️
channel ResourceIdentifier to a Channel Optional ResourceIdentifier<Channel> ✔️

CustomFieldDraft

field commercetools type required/optional JVM SDK draft type status
type ResourceIdentifier to a Type Required ResourceIdentifier<Type> ✔️

CategoryDraft

field commercetools type required/optional JVM SDK draft type status
parent ResourceIdentifier to a Category Optional ResourceIdentifier<Category> ✔️

InventoryEntryDraft

field commercetools type required/optional JVM SDK draft type status
supplyChannel ResourceIdentifier to a Channel Optional ResourceIdentifier<Channel> ✔️

CartDiscountDraft

field commercetools type required/optional JVM SDK draft type status
supplyChannel ResourceIdentifier to a Channel Optional ResourceIdentifier<Channel> ✔️

Not implemented yet

CustomerDraft

field commercetools type required/optional JVM SDK draft type status
stores Array of ResourceIdentifier to a Store Optional List<ResourceIdentifier<Store>> ✔️
customerGroup ResourceIdentifier to a CustomerGroup Optional ResourceIdentifier<CustomerGroup> ✔️

ShoppingListDraft

field commercetools type required/optional JVM SDK draft type status
customer ResourceIdentifier to a Customer Optional ResourceIdentifier<Customer> ✔️

@butenkor
Copy link
Member

As defined by @ahmetoz:

Support requests (SUPPORT-9797, SUPPORT-9798) are opened for the problematic types/fields both API and SDK and links are shared with @butenkor . Additionally, a GitHub issue is opened for jvm-sdk types: commercetools/commercetools-jvm-sdk#2052

@ahmetoz
Copy link
Contributor

ahmetoz commented Dec 29, 2020

The task is resolved, so only parts are related to the API, not SDK anymore, when API adds support of ResourceIdentifier for PriceDraft#customerGroup we could close the issue. (Currently, it's not clear when API will support this.)

@ahmetoz ahmetoz closed this as completed Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants