{"action":"created","issue":{"url":"https://api.github.com/repos/reactioncommerce/reaction/issues/3755","repository_url":"https://api.github.com/repos/reactioncommerce/reaction","labels_url":"https://api.github.com/repos/reactioncommerce/reaction/issues/3755/labels{/name}","comments_url":"https://api.github.com/repos/reactioncommerce/reaction/issues/3755/comments","events_url":"https://api.github.com/repos/reactioncommerce/reaction/issues/3755/events","html_url":"https://github.com/reactioncommerce/reaction/pull/3755","id":297397131,"number":3755,"title":"WIP: Fix: Setting or changing a products perma-link causes hard refresh","user":{"login":"prinzdezibel","id":1733229,"avatar_url":"https://avatars3.githubusercontent.com/u/1733229?v=4","gravatar_id":"","url":"https://api.github.com/users/prinzdezibel","html_url":"https://github.com/prinzdezibel","followers_url":"https://api.github.com/users/prinzdezibel/followers","following_url":"https://api.github.com/users/prinzdezibel/following{/other_user}","gists_url":"https://api.github.com/users/prinzdezibel/gists{/gist_id}","starred_url":"https://api.github.com/users/prinzdezibel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prinzdezibel/subscriptions","organizations_url":"https://api.github.com/users/prinzdezibel/orgs","repos_url":"https://api.github.com/users/prinzdezibel/repos","events_url":"https://api.github.com/users/prinzdezibel/events{/privacy}","received_events_url":"https://api.github.com/users/prinzdezibel/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-02-15T10:35:02Z","updated_at":"2018-02-15T16:19:53Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/reactioncommerce/reaction/pulls/3755","html_url":"https://github.com/reactioncommerce/reaction/pull/3755","diff_url":"https://github.com/reactioncommerce/reaction/pull/3755.diff","patch_url":"https://github.com/reactioncommerce/reaction/pull/3755.patch"},"body":"Resolves #2246\r\nImpact: minor\r\nType: bugfix\r\n\r\n## Issue\r\nChanging or creating the permalink of a product did trigger a page reload. Also this action was not reactive, meaning that other connected clients did not receive the new URL after publishing.\r\n\r\n## Solution/Changes\r\n\r\n- Don't trigger route change from callback value of meteor call `revisions/publish`, because\r\n1)  it's changing server state that gets delivered to connected clients asynchronously. It's not possible to rely on the publications already being synced to the client side when the method returns. This led to race conditions when trying to change the route client side, therefore it was done the hard way by setting `window.location.href`. Tried to observe permalink changes client side, but that was unreliable as well, probably because tracker runs may re-instantiate observed cursors at any time.\r\n\r\n2) it's not reactive (other connected clients won't get noticed about the route change), when the permalink has changed\r\n\r\n->  INSTEAD we observe changes to handles server side and let the client know it has changed through a new property `changedHandleWas`, which will be published like any other property (but doesn't get persisted to db)\r\n\r\n- ProductDetailContainer needs to subscribe to `Products`, as it is dependent on ReactionProduct.setProduct, which itself is dependent on`Products`.\r\n\r\n- Don't use window.location.href for route changes any more. Router.go() is used instead.\r\n\r\n- Changes to the Product publication which seems wrong  regarding updating product.__revision    \r\n  Don't publish the whole product object to the client. The code changed is only about the property __revisions on the product collection. Therefore it's wrong to push the whole object to the client. If done anyway, it will push product data to the client (e.g. handle, title) that is going to be updated soon after due to meteor method revision/publish, which results in **outdated** data on the client (which in turn results in template \"not found\" being displayed)\r\n\r\n    \r\n## Breaking changes\r\nnone expected\r\n\r\n\r\n## Testing\r\n1. As an admin create a new product\r\n2. Change permalink of the product\r\n3. Publish changes\r\n4. See updates to content and URL without page refresh\r\n5. URL for connected clients should update as well\r\n"},"comment":{"url":"https://api.github.com/repos/reactioncommerce/reaction/issues/comments/365978958","html_url":"https://github.com/reactioncommerce/reaction/pull/3755#issuecomment-365978958","issue_url":"https://api.github.com/repos/reactioncommerce/reaction/issues/3755","id":365978958,"user":{"login":"prinzdezibel","id":1733229,"avatar_url":"https://avatars3.githubusercontent.com/u/1733229?v=4","gravatar_id":"","url":"https://api.github.com/users/prinzdezibel","html_url":"https://github.com/prinzdezibel","followers_url":"https://api.github.com/users/prinzdezibel/followers","following_url":"https://api.github.com/users/prinzdezibel/following{/other_user}","gists_url":"https://api.github.com/users/prinzdezibel/gists{/gist_id}","starred_url":"https://api.github.com/users/prinzdezibel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prinzdezibel/subscriptions","organizations_url":"https://api.github.com/users/prinzdezibel/orgs","repos_url":"https://api.github.com/users/prinzdezibel/repos","events_url":"https://api.github.com/users/prinzdezibel/events{/privacy}","received_events_url":"https://api.github.com/users/prinzdezibel/received_events","type":"User","site_admin":false},"created_at":"2018-02-15T16:19:53Z","updated_at":"2018-02-15T16:19:53Z","author_association":"MEMBER","body":"Update: After more investigation, this gets really wicked (I'm sure this does not come as surprise to all guys having been working here already) !\r\n\r\nThis is my take away so far:\r\n1) I'm confident that this can be streamlined significantly, but it needs to be tested extensively\r\n2) I believe that the original ticket (changing the handle of a product with subsequent client side refresh without hard-reload) can't be achieved without addressing the following fact in the old code:\r\n  The **preSelector-cursor** might change when the user changes the permalink (`handle`): https://github.com/reactioncommerce/reaction/blob/ac07d02a971797863d0f593fc4608dd4b0a2611c/server/publications/collections/product.js#L93\r\n   This will have impact on the result of this cursor, that gets observed: https://github.com/reactioncommerce/reaction/blob/ac07d02a971797863d0f593fc4608dd4b0a2611c/server/publications/collections/product.js#L119\r\n3) I believe because server side publish functions are not reactive, the observer will not push changes to the client, if `Products.findOne(preSelector)` changes.\r\n\r\nI've tried to get rid of that cascading database query and it seems working so far. But this is no easy change and I'm not so naive to proclaim we're all good now!\r\n\r\n"}}
{"actor":{"display_login":"prinzdezibel"}}