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

Fix DFC catalog syncs for blank cart #13036

Conversation

chahmedejaz
Copy link
Collaborator

@chahmedejaz chahmedejaz commented Dec 12, 2024

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

What? Why?

What should we test?

  • In an order cycle with DFC products:
  • Check that stock adjusts after changes in Shopify after the user added a product to the cart.
  • If an item goes out of stock during checkout, the checkout should fail.

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

@chahmedejaz chahmedejaz added bugsnag api changes These pull requests change the API and can break integrations technical changes only These pull requests do not contain user facing changes and are grouped in release notes and removed api changes These pull requests change the API and can break integrations labels Dec 12, 2024
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I fixed one little typo but otherwise it's all good.

I always try to avoid send but it's the most pragmatic way here to avoid code duplication.

@mkllnk mkllnk marked this pull request as ready for review December 18, 2024 02:03
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@dacook
Copy link
Member

dacook commented Jan 2, 2025

I always try to avoid send but it's the most pragmatic way here to avoid code duplication.

I was thinking the same, but it's good to see deduplication.

Another way I could think of was to use a block, but it's not really worth it in this case.
Just to refresh myself on blocks, I thought I'd see how it looks:

def sync_catalogs(order)
  ...
  catalog_ids(order).each do |catalog_id|
     yield(user, catalog_id)
  end
 ...
end

def self.sync_linked_catalogs_later(order)
  sync_catalogs do |user, catalog_id|
    perform_later(user, catalog_id)
  end
end

.. err no, definitely not worth it!

@dacook
Copy link
Member

dacook commented Jan 2, 2025

@mkllnk can you comment if the testing team able is able to test this, or should it go to Garethe?

@mkllnk
Copy link
Member

mkllnk commented Jan 2, 2025

I think it's quickest if I test.

@mkllnk mkllnk added no-staging-UK A tag which does not trigger deployments, indicating a server is being used pr-staged-uk staging.openfoodnetwork.org.uk and removed no-staging-UK A tag which does not trigger deployments, indicating a server is being used pr-staged-uk staging.openfoodnetwork.org.uk labels Jan 2, 2025
@mkllnk
Copy link
Member

mkllnk commented Jan 2, 2025

The FDC Shopify API servers are shut down at the moment to save cost. Garethe will power them up again soon.

@RaggedStaff
Copy link
Collaborator

The FDC Shopify API servers are shut down at the moment to save cost. Garethe will power them up again soon.

@mkllnk - they're back up now.

@mkllnk mkllnk added the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 8, 2025
@mkllnk
Copy link
Member

mkllnk commented Jan 8, 2025

All works beautifully. Well done.

@mkllnk mkllnk merged commit 5f486bd into openfoodfoundation:master Jan 8, 2025
57 checks passed
@mkllnk mkllnk removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugsnag technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

DFC catalog syncs for blank cart
4 participants