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 API] Add custom OFN product id to DFC SuppliedProduct #11378

Merged
merged 7 commits into from Sep 22, 2023

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Aug 10, 2023

ℹ️ This is a funded feature. Please track your time in #9170 OFN DFC Products.

What? Why?

The DFC doesn't model grouping of several products (variants). So we add an OFN custom property for our own integrations. Other software should ignore it. For other semantic web tools the ofn: URI is not defined and can't be resolved. We haven't defined our own ontology. But I don't see a need for that at the moment.

What should we test?

  • Specs only

Release notes

Changelog Category: Technical changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@mkllnk mkllnk self-assigned this Aug 10, 2023
@mkllnk mkllnk added no-staging-FR A tag which does not trigger deployments, indicating a server is being used pr-staged-fr staging.coopcircuits.fr and removed pr-staged-fr staging.coopcircuits.fr no-staging-FR A tag which does not trigger deployments, indicating a server is being used labels Aug 10, 2023
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require "data_food_consortium/connector/connector"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
require "data_food_consortium/connector/connector"
require "datafoodconsortium/connector"

Copy link
Contributor

Choose a reason for hiding this comment

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

well, maybe not the best change :P will read better

@mkllnk mkllnk changed the title Add custom OFN product id to DFC SuppliedProduct [DFC API] Add custom OFN product id to DFC SuppliedProduct Sep 4, 2023
@mkllnk mkllnk added the technical changes only These pull requests do not contain user facing changes and are grouped in release notes label Sep 4, 2023
@mkllnk mkllnk force-pushed the dfc-ofn-product-group-id branch 2 times, most recently from 4b349ac to 5c33bab Compare September 20, 2023 07:04
When the application is not preloaded then running Rspec doesn't know
Rails until the spec helper is loaded. So we can't use Rails to find the
path of the spec helper. This has been fixed before but the DFC Address
code was developed at the same time and missed.
The Spree::Variant in OFN corresponds to a DFC SuppliedProduct. But
several Spree::Variant can be grouped under one Spree::Product which
wasn't exposed on the DFC API.

I'm adding a custom property here which can be used internally and
shouldn't break any other DFC tools.

A gotcha of this first test implementation:

The `ofn:` prefix has not been defined in the context. Software needs
to know that this is an Open Food Network attribute or ignore it.
We could define our own context and ontology and publish it on our
website but I don't see any benefit of that at this point.
In Rswag request specs, the `response` block is like `describe` and is
just used to group several other blocks. It can be long and that's okay.
@mkllnk mkllnk marked this pull request as ready for review September 20, 2023 23:21
@rioug
Copy link
Collaborator

rioug commented Sep 21, 2023

@mkllnk I am assuming this need to be tracked agains : #9170 OFN DFC Products ?

@mkllnk
Copy link
Member Author

mkllnk commented Sep 21, 2023

@rioug

I am assuming this need to be tracked agains : #9170 OFN DFC Products ?

Yes, that's right. I forgot to mention that.

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.

Great job! I have a small comment about formatting, but that is all.

Copy link
Collaborator

@rioug rioug 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 👍

@rioug
Copy link
Collaborator

rioug commented Sep 22, 2023

Merging as no manual test needed.

@rioug rioug merged commit b620e41 into openfoodfoundation:master Sep 22, 2023
50 checks passed
@mkllnk mkllnk deleted the dfc-ofn-product-group-id branch October 13, 2023 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Represent OFN Product on DFC API (not just variants)
4 participants