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
Concept of the shopping lists' line item update actions. #614
Merged
Merged
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
bbff90a
introduce adr,
ahmetoz 23e6a4e
add cases for the line item orders.
ahmetoz 3006072
update link.
ahmetoz 99b3a94
update.
ahmetoz da821b5
update.
ahmetoz 6d35149
Mention the importance of the order and addedAt value.
ahmetoz 1e9fccf
Update docs/adr/0002-shopping-lists-lineitem-and-textlineitem-update-…
ahmetoz b82c453
Update docs/adr/0002-shopping-lists-lineitem-and-textlineitem-update-…
ahmetoz 02fc905
Grammar updates
ahmetoz 3fba6c5
Add link to clarify the variant selection.
ahmetoz 7c9fc7a
update decision one.
ahmetoz 23e8208
mardown fix.
ahmetoz baa0c26
mention the action.
ahmetoz 6778f79
move notes to context.
ahmetoz b8f0947
fix typo, in quantity
ahmetoz 97d4d15
update wrong use case.
ahmetoz 8fe4121
use a different id to avoid confusion.
ahmetoz 76eb9d2
json fix.
ahmetoz 2a78606
Update docs/adr/0002-shopping-lists-lineitem-and-textlineitem-update-…
ahmetoz d4f24e6
add one more case.
ahmetoz 394c063
Merge remote-tracking branch 'origin/shopping-lists-base-branch' into…
ahmetoz 1ef3339
Update addedAt case.
ahmetoz c745ed0
Update addedAt field.
ahmetoz 3902376
Update the case 3.
ahmetoz 6096a78
Update cases for text line item.
ahmetoz 9d97f9a
Update docs/adr/0002-shopping-lists-lineitem-and-textlineitem-update-…
ahmetoz 18ce8ba
Update analysis.
ahmetoz e0e18bc
Apply suggestions from code review
ahmetoz ded6277
Reword, also..
ahmetoz 98822a9
make actions and decisions bold.
ahmetoz File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
docs/adr |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# 1. Record architecture decisions | ||
|
||
Date: 2020-11-04 | ||
|
||
## Status | ||
|
||
Accepted | ||
|
||
## Context | ||
|
||
We need to record the architectural decisions made on this project. | ||
|
||
## Decision | ||
|
||
We will use Architecture Decision Records, as [described by Michael Nygard](http://thinkrelevance.com/blog/2011/11/15/documenting-architecture-decisions). | ||
|
||
## Consequences | ||
|
||
See Michael Nygard's article, linked above. For a lightweight ADR toolset, see Nat Pryce's [adr-tools](https://github.com/npryce/adr-tools). |
363 changes: 363 additions & 0 deletions
363
docs/adr/0002-shopping-lists-lineitem-and-textlineitem-update-actions.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,363 @@ | ||
# 2. LineItem and TextLineItem update actions of the ShoppingLists. | ||
|
||
Date: 2020-11-04 | ||
|
||
## Status | ||
|
||
[Proposed](https://github.com/commercetools/commercetools-sync-java/pull/614) | ||
|
||
## Context | ||
|
||
<!-- The issue motivating this decision, and any context that influences or constrains the decision. --> | ||
|
||
In a commerce application, a shopping list is a personal wishlist of a customer, (i.e. ingredients for a recipe, birthday wishes). | ||
Shopping lists hold line items of products in the platform or any other items that can be described as text line items. | ||
|
||
We have challenges to build update actions of the `LineItem` and `TextLineItem` because of the nature of the synchronization, | ||
so in this document, we will describe the reasons and constraints, mostly related to order of the items: | ||
- LineItem orders might be important, if the customer has a front end that sorts the line items with their order could mean sorting by importance. | ||
- Similarly, `addedAt` data might be important if the customer has a front end that sorts the line items with the date. | ||
|
||
> Note: The cases below are the same for the `TextLineItem` so here those are not mentioned separately. | ||
|
||
### How to ensure line item order? | ||
|
||
<table> | ||
<tr> | ||
<th>LineItemDrafts</th> | ||
<th>LineItems</th> | ||
</tr> | ||
<tr> | ||
<td><pre lang="json"> | ||
{ | ||
"lineItems": [ | ||
{ | ||
"sku": "SKU-1", | ||
"quantity": 1, | ||
"addedAt": "2020-11-04T09:38:35.571Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-1" | ||
} | ||
} | ||
}, | ||
{ | ||
"sku": "SKU-2", | ||
"quantity": 2, | ||
"addedAt": "2020-11-04T09:40:12.341Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-2" | ||
} | ||
} | ||
} | ||
] | ||
} | ||
</pre> | ||
</td> | ||
<td><pre lang="json"> | ||
{ | ||
"lineItems": [ | ||
{ | ||
"id": "24de3821-e27d-4ddb-bd0b-ecc99365285f", | ||
"variant": { | ||
"sku": "SKU-2" | ||
}, | ||
"quantity": 2, | ||
"custom": { | ||
"type": { | ||
"id": "4796e155-f5a4-403a-ae1a-04b10c9dfc54", | ||
"obj": { | ||
"key": "custom-type-for-shoppinglists" | ||
} | ||
}, | ||
"fields": { | ||
"textField": "text-2" | ||
} | ||
}, | ||
"addedAt": "2020-11-04T09:38:35.571Z" | ||
} | ||
] | ||
} | ||
</pre> | ||
</td> | ||
</tr> | ||
<tr> | ||
<th colspan="2">Analysis</th> | ||
</tr> | ||
<tr> | ||
<td colspan="2"> | ||
<p>Draft has line items with <b>SKU-1</b> and <b>SKU-2</b>. In the target project line item with | ||
<b>SKU-2</b> exists, so <b>SKU-1</b> is a new line item. </p> | ||
<p> So we need to create an <a href="https://docs.commercetools.com/api/projects/shoppingLists#add-lineitem">AddLineItem</a> action | ||
and a <a href="https://docs.commercetools.com/api/projects/shoppingLists#change-lineitems-order">Change LineItems Order</a> | ||
of the line items <b>SKU-1</b> and <b>SKU-2</b>, because when we add line item with <b>SKU-1</b> | ||
the order will be <b>SKU-2</b> and <b>SKU-1</b>.</p> | ||
<p>The <b>challenge</b> in here is, those actions can not be added in one request because we don't know the | ||
line item id of the new line item | ||
with <b>SKU-1</b>, so we need to find another way to create a new line item with the right order.</p> | ||
</td> | ||
</tr> | ||
<tr> | ||
<th colspan="2">Proposed solution</th> | ||
</tr> | ||
<tr> | ||
<td colspan="2"> | ||
<p> | ||
Normally, for a difference, we might do a set intersection and then calculate action for differences, | ||
but that does not make sense because we are not aware of the order from the draft. | ||
So in this case the one request could be created but we might need to remove line item with <b>SKU-2</b> | ||
and line items in the draft with the given order with the line items <b>SKU-1</b> and <b>SKU-2</b>. | ||
</p> | ||
<pre lang="json"> | ||
{ | ||
"version": 1, | ||
"actions": [ | ||
{ | ||
"action": "removeLineItem", | ||
"lineItemId": "24de3821-e27d-4ddb-bd0b-ecc99365285f" | ||
}, | ||
{ | ||
"action": "addLineItem", | ||
"sku": "SKU-1", | ||
"quantity": 1, | ||
"addedAt": "2020-11-04T09:38:35.571Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-1" | ||
} | ||
} | ||
}, | ||
{ | ||
"action": "addLineItem", | ||
"sku": "SKU-2", | ||
"quantity": 2, | ||
"addedAt": "2020-11-04T09:40:12.341Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-2" | ||
} | ||
} | ||
} | ||
] | ||
} | ||
</pre> | ||
</td> | ||
</tr> | ||
</table> | ||
|
||
### Do we need to remove all line items when the order changes ? | ||
|
||
<table> | ||
<tr> | ||
<th>LineItemDrafts</th> | ||
<th>LineItems</th> | ||
</tr> | ||
<tr> | ||
<td><pre lang="json"> | ||
{ | ||
"lineItems": [ | ||
{ | ||
"sku": "SKU-1", | ||
"quantity": 1, | ||
"addedAt": "2020-11-04T09:38:35.571Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-1" | ||
} | ||
} | ||
}, | ||
{ | ||
"sku": "SKU-3", | ||
"quantity": 3, | ||
"addedAt": "2020-11-05T10:00:10.101Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-3" | ||
} | ||
} | ||
}, | ||
{ | ||
"sku": "SKU-2", | ||
"quantity": 2, | ||
"addedAt": "2020-11-04T09:40:12.341Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-2" | ||
} | ||
} | ||
} | ||
] | ||
} | ||
</pre> | ||
</td> | ||
<td><pre lang="json"> | ||
{ | ||
"lineItems": [ | ||
{ | ||
"id": "1c38d582-2e65-43f8-85db-4d34e6cff57a", | ||
"variant": { | ||
"sku": "SKU-1" | ||
}, | ||
"quantity": 1, | ||
"custom": { | ||
"type": { | ||
"id": "4796e155-f5a4-403a-ae1a-04b10c9dfc54", | ||
"obj": { | ||
"key": "custom-type-for-shoppinglists" | ||
} | ||
}, | ||
"fields": { | ||
"textField": "text-1" | ||
} | ||
}, | ||
"addedAt": "2020-11-04T09:38:35.571Z" | ||
}, | ||
{ | ||
"id": "24de3821-e27d-4ddb-bd0b-ecc99365285f", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it supposed to have different ID as LineItem SKU-1? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
"variant": { | ||
"sku": "SKU-2" | ||
}, | ||
"quantity": 2, | ||
"custom": { | ||
"type": { | ||
"id": "4796e155-f5a4-403a-ae1a-04b10c9dfc54", | ||
"obj": { | ||
"key": "custom-type-for-shoppinglists" | ||
} | ||
}, | ||
"fields": { | ||
"textField": "text-2" | ||
} | ||
}, | ||
"addedAt": "2020-11-04T09:40:12.341Z" | ||
} | ||
] | ||
} | ||
</pre> | ||
</td> | ||
</tr> | ||
<tr> | ||
<th colspan="2">Analysis</th> | ||
</tr> | ||
<tr> | ||
<td colspan="2"> | ||
<p>Draft has line items with <b>SKU-1</b>, <b>SKU-3</b> and <b>SKU-2</b> also in target project line item with | ||
<b>SKU-1</b> exists in the same order, <b>SKU-3</b> is a new line item, and <b>SKU-2</b> needs to be in last order.</p> | ||
<p> So we need to create an <a href="https://docs.commercetools.com/api/projects/shoppingLists#add-lineitem">AddLineItem</a> action and | ||
a <a href="https://docs.commercetools.com/api/projects/shoppingLists#change-lineitems-order">Change LineItems Order</a> | ||
of the line items <b>SKU-2</b> and <b>SKU-3</b>, because when we add line item with <b>SKU-3</b> | ||
the order will be <b>SKU-1</b>, <b>SKU-2</b> and <b>SKU-3</b>.</p> | ||
<p>The <b>challenge</b> in here is, those actions can not be added in one request because we don't know the | ||
line item id of the new line item | ||
with <b>SKU-3</b>, so we need to find another way to create a new line item with the right order.</p> | ||
<p>Also another <b>challenge</b> in here is about the line item with <b>SKU-1</b>, as the order and data is | ||
exactly same, we need to find a better way to avoid creating unnecessary actions. | ||
</p> | ||
</td> | ||
</tr> | ||
<tr> | ||
<th colspan="2">Proposed solution</th> | ||
</tr> | ||
<tr> | ||
<td colspan="2"> | ||
<p> | ||
The solution idea about the new line item and changed order is still same like in the case-1. Do we | ||
need to remove and add line item with <b>SKU-1</b>? No, it is not needed and we could start the removing | ||
and adding from the first order change. | ||
</p> | ||
<pre lang="json"> | ||
{ | ||
"version": 1, | ||
"actions": [ | ||
{ | ||
"action": "removeLineItem", | ||
"lineItemId": "24de3821-e27d-4ddb-bd0b-ecc99365285f" | ||
}, | ||
{ | ||
"action": "addLineItem", | ||
"sku": "SKU-3", | ||
"quantity": 3, | ||
"addedAt": "2020-11-05T10:00:10.101Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-3" | ||
} | ||
} | ||
}, | ||
{ | ||
"action": "addLineItem", | ||
"sku": "SKU-2", | ||
"quantity": 2, | ||
"addedAt": "2020-11-04T09:40:12.341Z", | ||
"custom": { | ||
"type": { | ||
"key": "custom-type-for-shoppinglists" | ||
}, | ||
"fields": { | ||
"textField": "text-2" | ||
} | ||
} | ||
} | ||
] | ||
} | ||
</pre> | ||
</td> | ||
</tr> | ||
</table> | ||
|
||
### How addedAt will be compared? | ||
|
||
In commercetools shopping lists API, there is no [update action](https://docs.commercetools.com/api/projects/shoppingLists#update-actions) | ||
to change the `addedAt` field of the `LineItem` and `TextLineItem`, also in API it has a default value described as `Defaults to the current date and time.`, | ||
when it's not set in the draft, so how to compare and update this field? | ||
|
||
**Proposed solution:** | ||
|
||
A sync option for excluding `addedAt` (based on the draft) comparisons, which defaults to `true`. The advantage of this, customers could decide themself sync `addedAt`. | ||
The drawback is when the option set to `false` and a difference is found by our utils, to ensure the order we need to remove and add all line items. | ||
|
||
## Decision (not agreed yet with the team) | ||
|
||
<!-- The change that we're proposing or have agreed to implement. --> | ||
|
||
- In commercetools API, the product variant to be selected in the LineItemDraft can be specified either by its product ID plus variant ID or by its SKU. | ||
For the sync library, product variant will be matched by its SKU, if the SKU not set for a LineItemDraft, the draft will not be synced and an error callback will be triggered. | ||
Check [LineItemDraft Product Variant Selection](https://docs.commercetools.com/api/projects/shoppingLists#lineitemdraft-product-variant-selection) for more details. | ||
|
||
- When a [Change LineItems Order](https://docs.commercetools.com/api/projects/shoppingLists#change-lineitems-order">) action is needed, | ||
ahmetoz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
the line items will be removed and added back with the order provided in the `ShoppingListDraft`. | ||
|
||
## Consequences | ||
|
||
<!-- What becomes easier or more difficult to do and any risks introduced by the change that will need to be mitigated. --> | ||
|
||
- To ensure the order, we need to remove and add line items, which means a bigger payload, so performance overhead. | ||
So as a result, we will not use the [Change LineItems Order](https://docs.commercetools.com/api/projects/shoppingLists#change-lineitems-order) update action. | ||
|
||
- [Add TextLineItem](https://docs.commercetools.com/api/projects/shoppingLists#add-textlineitem) is not checking if the data exists. A user could add the | ||
exact same data multiple times. It's hard to know the order by just checking the object. In this case, if the data is the same, the order will not be changed. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is quantity supposed to update? I see the quantity of existing lineitem 'sku-1' is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b8f0947