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

CategoryOrderHints are being imported before the product is linked with a category #163

Closed
junajan opened this issue Feb 21, 2017 · 12 comments
Assignees

Comments

@junajan
Copy link
Contributor

junajan commented Feb 21, 2017

Import should first link products with categories and then import categoryOrderHints (aka it should sort request actions):

{
  "statusCode": 400,
  "message": "Cannot set the order hint for the category 'ffa4b9b9-87bd-42c9-ac97-60da51c622f0' because the product '...' is not in that category.",
  "originalRequest": {
    "payload": {
      "actions": [
        {
          "action": "setCategoryOrderHint",
          "categoryId": "ffa4b9b9-87bd-42c9-ac97-60da51c622f0",
          "orderHint": "0.763"
        },
        {
          "category": {
            "typeId": "category",
            "id": "97153b1e-c6b3-4d81-98b3-02105ded377c"
          },
          "action": "removeFromCategory"
        },
        {
          "category": {
            "typeId": "category",
            "id": "ffa4b9b9-87bd-42c9-ac97-60da51c622f0"
          },
          "action": "addToCategory"
        }
      ],
      "version": 87
    }
  }
}
@junajan
Copy link
Contributor Author

junajan commented Feb 21, 2017

@Siilwyn @hisabimbola - this issue is not related only to this module so I was thinking that it should be a part of sphere-node-sdk? What do you think?

@Siilwyn
Copy link
Contributor

Siilwyn commented Feb 22, 2017

Could you check how the new SDK handles this?

@junajan
Copy link
Contributor Author

junajan commented Feb 22, 2017

Sure, will try it

@junajan
Copy link
Contributor Author

junajan commented Feb 23, 2017

@Siilwyn - I tested it on a ProductSync from SDK v1.17.6 with this code:

var sync = new (require('sphere-node-sdk').ProductSync)()
var _ = require('lodash')

var prod = {
  version: 1,
  "categories": [],
  "categoryOrderHints": {}
}
var prodNew = _.cloneDeep(prod)
prodNew.categories.push({
  "typeId": "category",
  id: "1234"
})
prodNew.categoryOrderHints["1234"] = "0.234"
var filtered = sync.config({})
.buildActions(prodNew, prod)

console.dir(filtered.getUpdatePayload(), { depth: 100 })

And the output is still wrong:

{ actions:
   [ { action: 'setCategoryOrderHint',
       categoryId: '1234',
       orderHint: '0.234' },
     { category: { typeId: 'category', id: '1234' },
       action: 'addToCategory' } ],
  version: 1 }

Action setCategoryOrderHint should be placed after addToCategory.

@Siilwyn
Copy link
Contributor

Siilwyn commented Feb 23, 2017

Whoops sorry Jan, I meant this one: https://github.com/commercetools/nodejs/tree/master/packages/sync-actions

@hisabimbola
Copy link
Contributor

Yeah, I think this fix should be done in the SDK, and new one too

@junajan
Copy link
Contributor Author

junajan commented Feb 27, 2017

@Siilwyn I tested it with the new ProductSync service and it did not even map categoryOrderHint actions:

var sync = require('@commercetools/sync-actions').createSyncProducts()
var _ = require('lodash')

var prod = {
  version: 1,
  categories: [],
  categoryOrderHints: {}
}
var prodNew = _.cloneDeep(prod)

prodNew.categories.push({
  typeId: "category",
  id: "1234"
})
prodNew.categoryOrderHints["1234"] = "0.234"

var filtered = sync.buildActions(prodNew, prod)
console.dir(filtered, { depth: 100 })

And the output is:

[ { category: { typeId: 'category', id: '1234' },
    action: 'addToCategory' } ]

@hisabimbola
Copy link
Contributor

We can fix this in the SDK...

@junajan
Copy link
Contributor Author

junajan commented Apr 5, 2017

This issue should be fixed by this PR: commercetools/sphere-node-sdk#226

@hisabimbola
Copy link
Contributor

This has been fixed in old sdk, we can keep this open till we fix in new sdk too...

@junajan
Copy link
Contributor Author

junajan commented May 16, 2017

PR which fixes this behaviour in the new SDK is here commercetools/nodejs#162

@junajan
Copy link
Contributor Author

junajan commented May 23, 2017

Closing this because all underlying PRs were merged.

@junajan junajan closed this as completed May 23, 2017
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

4 participants