Skip to content

Introduce Product Resource - #39

Open
imRohan wants to merge 2 commits into
mainfrom
rl-introduce-products
Open

Introduce Product Resource#39
imRohan wants to merge 2 commits into
mainfrom
rl-introduce-products

Conversation

@imRohan

@imRohan imRohan commented May 8, 2026

Copy link
Copy Markdown
Collaborator

The Core API supports retrieval of products by its Product ID. This PR
introduces the new Product resource, along with child resources needed
to build the main Product object. These child resources include
Allergens, Ingredient, Dietary Tag & Nutrition Fact.

This change addresses the need by:

  • Introducing a CoreObject resource which can be used by all core (v2)
    objects
  • Introducing a Product resource

@imRohan imRohan self-assigned this May 8, 2026
@imRohan
imRohan force-pushed the rl-introduce-products branch from b8686e4 to e4434cf Compare May 8, 2026 18:54
@imRohan

imRohan commented May 13, 2026

Copy link
Copy Markdown
Collaborator Author

Testing, working on staging (Auth + Retrieve Product)
image

@imRohan
imRohan force-pushed the rl-introduce-products branch 2 times, most recently from 83e0c90 to c99e65f Compare May 13, 2026 15:40
@imRohan
imRohan requested review from chriswoodford and jmazur May 13, 2026 18:06
@chriswoodford

Copy link
Copy Markdown
Contributor

the authentication changes need to be their own PR...

@imRohan

imRohan commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

This needs to be refactored after this PR is merged: #41

@imRohan
imRohan force-pushed the rl-introduce-products branch 2 times, most recently from ac7eff9 to 8b4e783 Compare August 10, 2026 18:13
@imRohan
imRohan requested a review from jmazur August 10, 2026 19:42
@imRohan
imRohan force-pushed the rl-introduce-products branch 3 times, most recently from f5022c0 to 8db3d7c Compare August 12, 2026 14:31
@imRohan
imRohan force-pushed the rl-introduce-products branch from dd091b3 to 2ba0d70 Compare August 18, 2026 15:06
Comment thread lib/bls/core/resources/product.rb Outdated
Comment on lines +17 to +21
as_packaged = NutritionFact.
build_from_array(data[:nutrition_facts][:as_packaged])
as_cooked = NutritionFact.
build_from_array(data[:nutrition_facts].
fetch(:as_cooked, []))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i guess as packaged always exists and as cooked only exists sometimes?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, exactly that. as_cooked is optionally returned, so using fetch to guard against that

@imRohan
imRohan force-pushed the rl-introduce-products branch from 2ba0d70 to df0a45b Compare August 25, 2026 17:17
The Core API supports retrieval of products by its Product ID. This PR
introduces the new Product resource, along with child resources needed
to build the main Product object. These child resources include
Allergens, Ingredient, Dietary Tag.

This change addresses the need by:
* Introducing a CoreObject resource which can be used by all core (v2)
  objects
* Introducing a Product resource
@imRohan
imRohan force-pushed the rl-introduce-products branch from df0a45b to 2943511 Compare August 25, 2026 17:26
Comment thread lib/bls/core/api/v1/authenticator.rb

module Bls
module Core
class DietaryTag < Bls::Core::ObjectV2; end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DietaryTag is kind of confusing naming... what is this and what is it used for?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's the naming which is returned by the core api, and is an array of diets which the meal abides by ex: ["PESCATARIAN"].

If the naming is not clear, perhaps the api should not return this key, and I can adjust this PR accordingly?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah it sounds like this needs to be thought about at the API level. make a note for you to revisit this with them. i think it's worth you reviewing naming across the entire product resource to comment on what might still sound sunbasket specific in the response

Comment thread lib/bls/core/resources/object_v2.rb
Comment thread lib/bls/core/resources/product.rb Outdated
Comment thread lib/bls/core/resources/product.rb Outdated
end

def self.build_from_response(data)
product = build(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pretty big method, consider extracting a factory class to use in here instead (will make testing way easier too)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you actually didn't write any tests for this...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Calling .retrieve on Product utilizes ApiOperations::Retrieve and if the response is successful it calls build_from_response.

Like other resources, I wrote tests for .retrieve and am testing to see if the object gets built successfully. But ill move to using a factory class

Comment thread spec/support/fixtures/product_response.json Outdated
@imRohan
imRohan requested a review from chriswoodford August 26, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants