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

✨🥗 Marketplace: VendorRepresentative manages Products #2181

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

zspencer
Copy link
Member

@zspencer zspencer commented Feb 7, 2024

OK this should probably be split into a number of smaller steps, each with reasonable test coverage; but I wanted to get something working so we can turn the keys over to the folks at Oaklandia.

This makes it so:

  • A Neighborhood Operator or Space Member can add a VendorRepresentative to a Marketplace
  • The VendorRepresentative can add, remove, and update Products

YOLO YOLO YOLO

@zspencer zspencer requested review from a team February 7, 2024 04:57

belongs_to :marketplace, inverse_of: :vendor_representatives
has_one :room, through: :marketplace
belongs_to :person
Copy link
Contributor

Choose a reason for hiding this comment

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

Record creation is failing because this is not optional. Observed this while playing with the branch locally.

Copy link
Member Author

Choose a reason for hiding this comment

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

Weird; I was able to create a VendorRepresentative through the UI...

I'll add a spec/marketplace/system/vendor_representatives_system_spec.rb and a bit more tests tonight before merging.

Copy link
Member

Choose a reason for hiding this comment

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

Certainly as written now this class requires to be associated to a person to be valid.

Copy link
Member Author

Choose a reason for hiding this comment

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

yea, adding the spec caused it to fail until I put this in... Now I'm wondering why I could do it last night...

@rosschapman
Copy link
Contributor

@zspencer just want to confirm that a vendor rep must be claimed before they actually have any write access to products etc... right? I think that workflow makes sense. Also a Person is generally created through an invitation to membership, right?

- #2044

OK this should probably be split into a number of smaller steps, each
with reasonable test coverage; but I wanted to get something working so
we can turn the keys over to the folks at Oaklandia.

This makes it so:

- A `Neighborhood` `Operator` or `Space` `Member` can add a
  `VendorRepresentative` to a `Marketplace`
- The `VendorRepresentative` can add, remove, and update `Products`

YOLO YOLO YOLO
@zspencer zspencer force-pushed the marketplace/vendor/adding-a-vendor branch from dcc6e68 to 1768518 Compare February 8, 2024 00:43
Copy link
Member

@anaulin anaulin left a comment

Choose a reason for hiding this comment

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

I guess this is fine if it gets the job done.

I'm a bit confused about the intended workflow here. So the marketplace owner adds an owner representative, then talks to them to set up an account, then goes back to the marketplace and "claims" them, is that the intended flow?

Why not do something simpler, where a VendorRepresentative is just a bridge between a marketplace and a person, and the Marketplace owner can add it by selecting from a list of People who are members of the Space? (and if the person they want to add is not a member, they can send an invitation, making that person a member, right?)

@zspencer
Copy link
Member Author

zspencer commented Feb 8, 2024

Why not do something simpler, where a VendorRepresentative is just a bridge between a marketplace and a person, and the Marketplace owner can add it by selecting from a list of People who are members of the Space? (and if the person they want to add is not a member, they can send an invitation, making that person a member, right?)

Right now, being a Member grants wide permissions across the Space; we want to keep the permissions to only the Marketplace. So we can't add a Person from their Membership, since that would escalate their permissions too highly.

I'm a bit confused about the intended workflow here. So the marketplace owner adds an owner representative, then talks to them to set up an account, then goes back to the marketplace and "claims" them, is that the intended flow?

I wanted to figure out a way to link it directly and not have to do this awkward handshake; but I couldn't come up with a way to do it without pushing the Vendor into Marketplace land. It's not ideal :(.

@zspencer zspencer added the ✨ feature Reduces Client's Burden or Grants them Benefits label Feb 8, 2024
@zspencer
Copy link
Member Author

zspencer commented Feb 8, 2024

@zspencer just want to confirm that a vendor rep must be claimed before they actually have any write access to products etc... right? I think that workflow makes sense. Also a Person is generally created through an invitation to membership, right?

Yes, a VendorRepresentative must be associated to a Person before they can do anything in the Marketplace.

A Person is created whenever someone signs in to a Space in the Neighborhood. We probably want to move so that "Person registers in Neighborhood" is it's own distinct workflow tho, since I now understand the folly of conflating sign up and sign in. Plus the interaction design considerations for appropriately signlaing that people are registering for a broad set of websites/apps vs just the one website/app...

@zspencer zspencer changed the title Marketplace: VendorRepresentative manages Products ✨🥗 Marketplace: VendorRepresentative manages Products Feb 8, 2024
@zspencer zspencer merged commit ff81597 into main Feb 8, 2024
3 checks passed
@zspencer zspencer deleted the marketplace/vendor/adding-a-vendor branch February 8, 2024 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature Reduces Client's Burden or Grants them Benefits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants