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.

Sign up for GitHub

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

Jump to bottom

feat: only show shared suppliers that are allowed to be shown #1036

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: only show shared suppliers that are allowed to be shown #1036

wants to merge 1 commit into from

Conversation

kidhab
Copy link
Contributor

@kidhab kidhab commented Dec 22, 2023

This SharedLists commit introduces a new foodcoop column at the SharedLists suppliers table.

The web interface will show a field that expects the name of a Foodcoop (has to be defined via [FoodsoftConfig[:name]). If the field is empty, the supplier will be show to all Foodsoft multisite instances.

If the field contains a string the Foodsoft will use it to filter out suppliers that are only allowed to be shown for a specific Foodcoop instance.

This MR introduces the filter at the Foodsoft side.

Closes #1033

@kidhab
Copy link
Contributor Author

kidhab commented Dec 22, 2023

Maybe it would be better to introduce a new config item because it's possible to set [FoodsoftConfig[:name]) via the web interface (and not only via app_config.yml).
If someone changes the value via web this would filter out the supplier.

Anyway let me know what you think in general.

@yksflip
Copy link
Member

yksflip commented Dec 28, 2023

hey,
as already mentioned in #1033, we should talk about how we want to proceed with sharedlist in general. I think the suggestion made by lentschi look very good.
But as the timeline of the foodsoftAT fork is not predictable yet, I'd be okay to go with your changes, so you don't have to wait for it to finish

@yksflip
Copy link
Member

yksflip commented Dec 28, 2023

Maybe you can have another look at the tests, it look's like your changes broke something.

@wvengen
Copy link
Member

wvengen commented Apr 8, 2024

We could use the foodcoop config key (I think it is sometimes also called 'scope') instead of name. It's not ideal, I would expect a one-to-many relation (being able to allow multiple foodcoops), but as this may be a temporary solution anyway - until we have restructured the shared database approach (which will probably take some time) - it sounds like a great simple approach to solve a relevant use-case.

@kidhab
Copy link
Contributor Author

kidhab commented Apr 9, 2024

Maybe you can have another look at the tests, it look's like your changes broke something.

Can you re-run the failed job so that I can see the error message again? I don't have the necessary permissions.

@wvengen
Copy link
Member

wvengen commented Apr 9, 2024

Can you re-run the failed job so that I can see the error message again? I don't have the necessary permissions.

It appears this run has expired, and needs a re-push to run CI again (and if that fails, we could try to re-run in case of a flaky test).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Access control for shared suppliers
3 participants
@kidhab @yksflip @wvengen