openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.1k stars 714 forks source link

As a dedicated user, I can access a DFC API endpoint with fewer data and all enterprises #12544

Closed RachL closed 1 week ago

RachL commented 3 months ago

Funded feature code is #12543 [DFC] Anonymized orders endpoint

Description

- As a: enterprise user - On page: on the DFC API - I want to be able to do: see an order endpoint with fewer data than the regular DFC order endpoint.

See data format specification: https://app.swaggerhub.com/apis-docs/RAGGEDSTAFF23/DFC-INRAE-Endpoint/v0.1#/INRAE/get_api_dfc_Persons__id__AffilateSalesData

This endpoint needs to reveal only

And ideally only for a given period and a list of product names:

Technical tasks

Acceptance Criteria & Tests

  1. Turn on Flipper toggle
  2. Allow one email address in the toggle
  3. Log in with that email address
  4. See that this endpoint gives you the list of data detailed above, for all enterprises on the database
  5. Enterprise name and full address can't be found
RaggedStaff commented 3 months ago

Ok. I think I see the issue... this isn't an Orders endpoint... you aren't sharing data from individual Orders, you're aggregating Orders, from multiple Enterprises, for a given reporting period (to/from date).

I'm on my phone atm, but can look when I'm back on the laptop & provide dfc mappings for the ofn fields you've mentioned.

I would suggest a name like AggregatedSalesData

You probably want to feed in the date range as parameters (either in path or in query).

Wondering how efficient OFN is at builsing this resultset on the fly... do we need to consider indexing/performance? 🤔

RaggedStaff commented 2 months ago

As @mkllnk already mentioned...

dfc-b:PostCode is accesible through Enterprise:hasAddress (inherited from Agent, so you might need to look there)

Product/Variant Units are parsed via the measures.rdf SKOS vocabulary. Not sure how that was mapped, but you should be able to yoink the same code as for the Product endpoints.

Price should be passed as a dfc-bPrice object, which is attached to an Offer, which is linked to a CatalogItem (hence why we tend to serve SuppliedProduct with CatalogItem). You'll need to embed the links in the graph, but not the entire object, just @id will do.

Offer also links to SaleSession which will be where you can get the Orders link & determine the quantity sold. I think you'll need to pass all the OrderLines to the object, any aggregation would need to happen after the DFC objects are parsed.

@anansilva - Hope that all makes sense, I'll give you a buzz on Slack.

RaggedStaff commented 2 months ago

Further thoughts...

To make the graph hang together, I think we need to start from a User (the INRAE user), affilitate them to all the Enterprises that are willing to share data & walk the graph to OrderLines from there.

dacook commented 1 month ago

Regarding authorisation:

  • The email address can be authorized against our feature toggles. For example Flipper.enable(:inrae, "jo@inrae.org"). Then we can create a model class to work with Flipper: InraeUser = Struct.new(:flipper_id) and pass that on to check if the email is allowed to access this endpoint.

I didn't realise this before, so I implemented this a bit differently, on the user model: https://github.com/openfoodfoundation/openfoodnetwork/pull/12676/files?w=1#diff-08a39940388ebab42d9f8314c302c8bb8493b8eda6216fd9c8909e5a9756ae8a

It means that we can create an INRAE user in OFN, then grant them access to the affiliate_sales_data feature with the normal "actor" method (eg Spree::User;1234).

mkllnk commented 1 month ago

@RachL, is that authentication okay? I thought that they didn't want a user account but maybe it doesn't matter?

RachL commented 1 month ago

@mkllnk @dacook I'm sorry if there was a confusion, there is no issue with creating them an account. What we didn't want was to have to add them as managers of enterprises.

So using the "actor" method sounds good to me :+1:

mkllnk commented 2 weeks ago

@RachL, I pickup this issue and made some good progress but I'm unsure about some details. Do you have an example from Socleo?

@RaggedStaff created an example which I'm following. There, the distributor is expressed as the coordinator of the sale session. In OFN that would be the coordinator of the order cycle. But the sale is actually through the distributor and their shop front (outgoing exchange of the order cycle). That would correspond to an enterprise having catalog items with offers referenced in the order lines.

I'll just follow the example for now but wanted to raise that there may be some room for interpretation and we need to match what Socleo does.

RachL commented 2 weeks ago

@mkllnk yes here it is: https://demo.socleo.org/api/dfc/inrae/person/187432?startDate=01/01/2024&endDate=31/12/2024 Headers : authorization:JWT {access_token}

mkllnk commented 2 weeks ago

Thank you. That's a completely different graph to Garethe's example. :disappointed:

It also preserves the association of customers to orders, not sure if the customer ids have any relevance though.

There are offers in the graph with price but I think that INRAE just wants to know the sold price on the line item, which is included as well.

I'll not redo my work now until INRAE tried to process this format. Maybe they can tell us better what they need.

mkllnk commented 2 weeks ago

@RachL Another query about the opt-in:

I filtered by distributor so far. Do we need to filter by supplier as well?

My logic was that we are looking at sales and the sale is done by the distributor. They can set their own prices in the inventory. But if a supplier doesn't want their products to be part of the study then maybe they want to opt out?

I think it's only a ten minute change.

mkllnk commented 2 weeks ago

@RachL, here's my implementation:

Feel free to stage it, play around or supply the data to INRAE. Any response yet?

RachL commented 1 week ago

Thank you. That's a completely different graph to Garethe's example.

Oh no :( I don't understand how it is possible... I need to contact them.

It also preserves the association of customers to orders, not sure if the customer ids have any relevance though.

To my understanding they uses customers to fetch their postcode (as the distributor postcode is missing from the model) and they use it as a distributor postcode (assuming the location of the customer is the location of the distributor which I think is the case in 99% of the time).

I'll not redo my work now until INRAE tried to process this format. Maybe they can tell us better what they need.

Completely aligned

I filtered by distributor so far. Do we need to filter by supplier as well?

I'm not sure I understand sorry, do you have an example?

Feel free to stage it, play around or supply the data to INRAE. Any response yet?

Thank you so much! I will test today. No response yet, but they are back from holiday. I guess they haven't look at

RachL commented 1 week ago

(sorry I wrote this ☝️ last friday but apparently I didn't press the comment button 🤦‍♀️ )

mkllnk commented 1 week ago

I filtered by distributor so far. Do we need to filter by supplier as well?

I'm not sure I understand sorry, do you have an example?

Enterprises can opt in/out via the connected apps tab. And I'm assuming that distributors are opting out to not be included in the sales data given to INRAE. But we could also filter by supplier so that even if a distributor is opting in, some of the sales data is filtered out if the products come from a supplier who who opted out.