search

openfoodfoundation / openfoodnetwork

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

[Admin][Permissions] Relationships between enterprises not displayed correctly #9793

Open drummer83 opened 1 year ago

drummer83 commented 1 year ago

Description

This is a follow up of #9768.

EDIT: !1! and !2! are covered by #302 now. Only !3! remains open in this issue here.

While testing #9768 systematically, more issues going beyond #9750 were identified. The examples used the following permissions: image

The identified issues are: !1! Producer shop should not be displayed as a place to buy producer profile's or another producer shop's or producer hub's products because by definition a producer shop only sells own products. image

!2! Producer profile or another producer shop or producer hub should not be displayed as a producer of producer shop's products, because by definition the producer shop only sells own products. image

!3! A granted permission to add to order cycle of B, but B didn't grant permission to A, so A shouldn't be listed as a place to buy B's products. image

These relationships can be seen on the following pages:

  1. Shop/hub modal (on page /map or when clicking on a producer in the shopfront) image
  2. Profile modal (on page /map or when clicking on a producer in the shopfront) image
  3. Our producers list (on page /shops) image
  4. Vendors list (on page /producers -> shop for x's products at shop xyz) image
  5. Producers list (in the shopfront of an enterprise) image
A systematical test has been carried out and identified the following problems: Permission Shop/hub modal Profile modal Shops -> our producers Producers -> Shop for ... Shop -> Producers
producer profile grants permission to producer profile - OK OK OK -
producer profile grants permission to producer shop OK !1! :x: !2! :x: !1! :x: !2! :x:
producer profile grants permission to producer hub OK OK OK OK OK
producer profile grants permission to non-producer profile - OK - OK -
producer profile grants permission to non-producer hub OK OK OK OK OK
producer shop grants permission to producer profile OK !1! !3! :x: OK !1! !3! :x: OK
producer shop grants permission to producer shop !1! !3! :x: - !2! :x: !1! !3! :x: !2! :x:
producer shop grants permission to producer hub !1! !3! :x: - OK !1! !3! :x: OK
producer shop grants permission to non-producer profile OK OK OK OK OK
producer shop grants permission to non-producer hub OK - OK OK OK
producer hub grants permission to producer profile OK !3! :x: OK !3! :x: OK
producer hub grants permission to producer shop !1! !3! :x: - !2! :x: !1! !3! :x: !2! :x:
producer hub grants permission to producer hub !3! :x: - OK !3! :x: OK
producer hub grants permission to non-producer profile OK OK OK OK OK
producer hub grants permission to non-producer hub OK - OK OK OK
non-producer profile grants permission to producer profile - OK - OK -
non-producer profile grants permission to producer shop OK OK OK OK OK
non-producer profile grants permission to producer hub OK OK OK OK OK
non-producer profile grants permission to non-producer profile - OK - - -
non-producer profile grants permission to non-producer hub OK OK OK - OK
non-producer hub grants permission to producer profile OK !3! :x: OK !3! :x: OK
non-producer hub grants permission to producer shop OK !3! :x: OK !3! :x: OK
non-producer hub grants permission to producer hub !3! :x: - OK !3! :x: OK
non-producer hub grants permission to non-producer profile OK OK OK OK OK
non-producer hub grants permission to non-producer hub OK - OK OK OK

Expected Behavior

The relationships should be displayed correctly.

Actual Behaviour

Producer shops are displayed as places to by other producer's products. Enterprises are displayed as producers of a producer shop's products. Permissions are applied in both directions while they should only be applied in one direction.

Steps to Reproduce

  1. Set up permissions between the enterprises of interest.
  2. Check the 5 places where permissions are displayed in the app.

Animated Gif/Screenshot

See above.

Workaround

None.

Severity

bug-s4: it's annoying, but you can use it

Your Environment

Possible Fix

RachL commented 1 year ago

@drummer83 I'm a bit confused by your screenshots. On the first we can see that A grant permission to B. So with this in mind. Case 1,2,3 are correct?

drummer83 commented 1 year ago

@RachL I don't think !1!, !2! and !3! are correct.

Regarding !1! and !2!: It is important to keep in mind that both enterprises in this case are producer shops (sells own). If one of them sold products of another producer it would become a producer hub (sells any) by definition. To make it short:

Regarding !3!: I had A and B mixed up in the wording. This is now corrected: !3! A granted permission to add to order cycle of B, but B didn't grant permission to A, so A shouldn't be listed as a place to buy B's products.

Makes sense?

RachL commented 1 year ago

@drummer83

If one of them sold products of another producer it would become a producer hub (sells any) by definition.

By definition, yes. But the display on the profile would still be cases 1,2. The problem here is not the display on the profile. It's just that when a producer sells "own" and gives permission to another shop to sell, we should switch its settings to "any".

The profile display is matching the permissions so these cases look correct to me.

Ok for case 3. But it's weird I can't reproduce it in production. Perhaps there are more stuff coming into play to reproduce this one?

drummer83 commented 1 year ago

@RachL I think we both agree that there is something wrong here: either the display or the setting of 'sells'. Possible solutions to solve this are:

Both is fine for me - no strong opinion.

Re. !3!: Maybe it's the visibility setting of your enterprises?

RachL commented 1 year ago

B: The permissions overrule/set the 'sells' setting and the display is rendered according to the permissions.

@drummer83 but just to be sure, it looks to me that this solution is already in place, right?

drummer83 commented 1 year ago

Uhmm, not so sure. I didn't focus on this during testing. We definitely don't have the functionality that adding permissions actually sets the 'sells' setting, e.g. from producer shop to producer hub.

RachL commented 1 year ago

No but it looks to me like permissions do override the setting and perhaps we should leave it this way, as it's easier for the user. So we could open another issue in order to switch the setting when more permissions are added?

drummer83 commented 1 year ago

Yes, I will open that as a wishlist item and edit here that !1! and !2! will we covered in that one.