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

[DFC Products] Ensure Product details are synced at correct intervals #12986

Open
RaggedStaff opened this issue Nov 19, 2024 · 12 comments · May be fixed by #13167
Open

[DFC Products] Ensure Product details are synced at correct intervals #12986

RaggedStaff opened this issue Nov 19, 2024 · 12 comments · May be fixed by #13167
Assignees

Comments

@RaggedStaff
Copy link
Collaborator

RaggedStaff commented Nov 19, 2024

ℹ️ Funded Feature. Please track ALL ASSOCIATED WORK under the associated tracking code #11678 DFC Orders

We need to ensure Product details are automatically updated when appropriate, and not when not appropriate.

There is an agreement within FDC that only Stock details will be updated within an Order Cycle (Sales Session).

All other details (image, description, title, etc) should be refreshed between OC's (potentially when setting up an OC and at the start of every OC)

Requirements

  • When exactly shall we trigger the update? when the order cycle opens.
  • we can do a full refresh/update of the product at that point (including details, prices and stock).
@RaggedStaff

This comment has been minimized.

@mkllnk
Copy link
Member

mkllnk commented Nov 29, 2024

That makes sense, vaguely, but it's missing some details.

  • When exactly shall we trigger the update? I propose to do it when the order cycle opens. We already trigger webhooks at that time and can also trigger the update job. If you want to test the outcome first, you can import the catalog manually.
  • Is there any data we don't want to update? I think that we should still update stock levels as well.
  • Is it okay if the product data is updated during an order cycle? If you have multiple overlapping order cycles with the same products then the syncing would happen for every order cycle opening. So when the second order cycle opens, the first one is already in full swing and prices could change. Is that okay?
  • We need to optimise image updating. I noticed that we update the product image every time we update even if it's still the same image. I think that we should store the original URL as metadata with our images and only update if that changes. Does that make sense to you?

Estimate: 2 days

@RaggedStaff
Copy link
Collaborator Author

When exactly shall we trigger the update? I propose to do it when the order cycle opens.

Yes, that seems best for now... I'd like to get to a point where the product import/selection could be embedded in the OC setup process but we need the work on the import first & this is more urgent.

Is there any data we don't want to update? I think that we should still update stock levels as well.

No, I think we can do a full refresh/update at that point.

Is it okay if the product data is updated during an order cycle? If you have multiple overlapping order cycles with the same products then the syncing would happen for every order cycle opening. So when the second order cycle opens, the first one is already in full swing and prices could change. Is that okay?

Hmm, that's not ideal. Curse OFN & it's ridiculously adaptable model ! 😖 For now (current pilots) It won't be an issue (we're already having to set up multiple Enterprises to import Products into to handle different discount fees for Wholesale vs Retail products. Maybe (for now) we can add a warning to the (successful) Import splash screen to say "Don't use these Products in overlapping Order Cycles" or something similar. WDYT ?

We need to optimise image updating. I noticed that we update the product image every time we update even if it's still the same image. I think that we should store the original URL as metadata with our images and only update if that changes. Does that make sense to you?

🤔 Not sure it required that the URL necessarily changes, everytime the image is updated.

@sigmundpetersen sigmundpetersen changed the title [DFC Product] Ensure Product details are synced at correct intervals [DFC Products] Ensure Product details are synced at correct intervals Dec 3, 2024
@mkllnk mkllnk assigned dacook and unassigned mkllnk Feb 11, 2025
@dacook
Copy link
Member

dacook commented Feb 12, 2025

Hi @RaggedStaff and Maikel. The first couple of points discussed seem clear so I've updated the description.

Overlapping order cycles: that's a tricky one. I think the biggest problem is that we're doing something automatically, in the background, and there's no indication of this to the shop manager. Some ideas:

  1. Send an email to shop managers when the auto update happens. This will serve to notify them to confirm what has happened, and should explain why (IE because the product is linked to a remote product).
  2. Display something on the admin product screen, to reveal that the product is linked to a remote product. There could be an info ℹ icon with tooltip explaining more (and perhaps reveal the uid also).
  3. Display something on the order cycle admin screen. IE when there are remote products in the order cycle, show a notice saying so, and that products will be synced at order cycle open.
    • Perhaps this could be extended further in the future to provide an option on the order cycle to enable/disable this feature.

I think option 1 should be implemented as part of this issue. What do you think? Any suggestion for the email copy?

Optimise image updating: good point about the URL not necessarily changing. Ideally I think the DFC should have an attribute for the hash of the file, so you can see if it's changed or not. Is that possible? Other thoughts:

  • If we implemented caching based on URL, we could get the remote service to provide unique info as part of the image url, eg a cachebusting hash or timestamp: http://example.com/product1.jpg?updated=20250101... But we still would want to expire our cache somehow, in case the URL doesn't change.
  • an DFC attribute for file modified date would be helpful, if hash is too hard.

This seems non-essential to this task, so perhaps we create separate issue for it.

I'll make a start on the basic requirements.

@dacook dacook moved this from All the things 💤 to In Progress ⚙ in OFN Delivery board Feb 12, 2025
@dacook
Copy link
Member

dacook commented Feb 12, 2025

@mkllnk , while working through this in more detail I decided:

  • We shouldn't consider an order cycle "open" until all relevant products have been synced. This might take a while, so the order cycle might not open until a minute after the open time.

Would that be ok do you think?
Also, is it possible to update products individually, or would we need to fetch a catalogue and use that for the import?

This might make more sense in (pseudo) code: dacook@bfb6b6a

@mkllnk
Copy link
Member

mkllnk commented Feb 13, 2025

David and I just discussed that we'll keep the minimal possible scope and just check at the beginning of one order cycle if there's another one open already. And if so, we don't update the the product details.

We'll wait for user input before deciding about sending emails in this case or sending general notifications on sync. We may as well find it's better to have a sync button on the order cycle edit page? @RaggedStaff

@mkllnk
Copy link
Member

mkllnk commented Feb 13, 2025

Ideally I think the DFC should have an attribute for the hash of the file, so you can see if it's changed or not. Is that possible?

That would be nice. We could add it to the standard but it'll take a while to be in production. And some implementations may not define that attribute either.

So for now I propose to store the URL on the ActiveStorage::Blob as metadata (supported by Rails) and assume that the URL will change. At least for the current integration with Shopify that's true. And when an app comes along that doesn't change the URL when the image is updated then we can look at new solutions. But it will probably be easier for that app to add a timestamp to the URL as you suggested than computing a checksum.

So after thinking this through, I would just say that the DFC requires the URL to change when the image changes.

@mkllnk
Copy link
Member

mkllnk commented Feb 13, 2025

I started work on the image issue:

@dacook
Copy link
Member

dacook commented Feb 17, 2025

We may as well find it's better to have a sync button on the order cycle edit page?

Without knowing the context more, I think this sounds like it could be a reasonable alternative to it being automatic.
So I'll pause until Garethe's available to comment, unless @mkllnk can say with confidence which way to go?

@dacook
Copy link
Member

dacook commented Feb 18, 2025

Just discussed with Maikel, we think the automation is probably the end goal, given that the products should be considered a cache of the remote source of truth. So I'll proceed with that.

@RaggedStaff
Copy link
Collaborator Author

Sorry folks, catching up on this.

Yes, automation is the goal. If there aren't open OC's, could we also refresh when the products are added to an OC ? That might prevent confusion around "this product doesn't look up to date" or "I thought the price was still X, cos that's what it said when I put it in the OC" .

I like the idea of a tooltip displaying the URI of the remote product (with an explanation 😉 )

@mkllnk
Copy link
Member

mkllnk commented Feb 18, 2025

If there aren't open OC's, could we also refresh when the products are added to an OC ?

That needs some UX consideration. Syncing the catalog can take several seconds. So it's probably not something we want to do on page load unless we have some clear messaging like a spinner: "Just a moment, we are syncing your product catalog." Or we come back to a button on that page. So if you think that the information is out of date then you can hit the Refresh product data button.

I reckon that we add that as new issue to avoid the scope creep here. Would you like that?

@dacook dacook linked a pull request Feb 19, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Status: In Progress ⚙
Development

Successfully merging a pull request may close this issue.

3 participants