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

Gambits Management improvement #2306

Closed
iPurpl3x opened this issue Sep 24, 2020 · 6 comments
Closed

Gambits Management improvement #2306

iPurpl3x opened this issue Sep 24, 2020 · 6 comments
Labels
stale Issues that have had over 90 days of inactivity

Comments

@iPurpl3x
Copy link
Contributor

Feature Request

I've run into a situation where it would have been really handy to know the list of active Gambits while inside of one of these Gambits. As I found out, the code (usually the conditions() method) of the gambits is executed at the same time as the list of active gambits is created in the GambitsManager. This means if you check for $search->activeGambits() you'll get []...

I would propose to make changes to the GambitManager (and maybe more if needed) to run a 2-step process that first checks what gambits are active and only then runs their conditions.

FYI:
In my case I found a workaround where I would use a Middleware and a Provider to make the current $request object available with app('request') from anywhere in the code. Having that, I was able to mimic some code of the GambitManager to check what Gambits are active before the list would get populated.

@franzliedke
Copy link
Contributor

Could you please share more details about your use-case so that we can understand why you need this?

If we really want to allow this (and currently it just feels like a super rare edge-case), we would probably have to split up the gambit's apply() method into two methods: one for checking whether it can (and will) be applied, and another one for actually applying it.

@iPurpl3x
Copy link
Contributor Author

iPurpl3x commented Oct 6, 2020

@franzliedke Here are some details:

I'm writing an extension that uses discussions as data container for entities that we call reviews (they are connected to a product, in this case a book from a book store. These reviews live alongside of discussions in the forum and I use a gambit to tell the backend to either filter out the regular discussions or the reviews. This work pretty well.

Now I'm working on the /following page and there things are getting very tricky...
Somehow my filtering doesn't work all the time and I found out that if I add some filtering to the subquery that is done by the flarum/subscriptions extension (https://github.com/flarum/subscriptions/blob/master/src/Gambit/SubscriptionGambit.php#L31) then it works, but only in one way: I can filter out the reviews to only have the regular discussions, but I can't do the opposite because I don't know if the request asks for regular discussions or for reviews. I could do this properly if I had access to the active gambits because then I would know what to filter.

@askvortsov1
Copy link
Sponsor Member

Tbh, I feel like the most effective solution would be to just introduce a parallel endpoint for searching reviews, and add it as a search source in the frontend. I don't think we have any provisions for "discussion types"

@iPurpl3x
Copy link
Contributor Author

iPurpl3x commented Oct 6, 2020

Tbh, I feel like the most effective solution would be to just introduce a parallel endpoint for searching reviews, and add it as a search source in the frontend. I don't think we have any provisions for "discussion types"

Maybe I'll have to consider this. I initially didn't do it with a separate endpoint because I wanted to reuse as much as possible from the discussions, including features that come from extensions. Both roads lead into difficulties and maintenance problems...

@stale
Copy link

stale bot commented Jan 10, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Jan 10, 2021
@stale
Copy link

stale bot commented Feb 9, 2021

We are closing this issue as it seems to have grown stale. If you still encounter this problem with the latest version, feel free to re-open it.

@stale stale bot closed this as completed Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that have had over 90 days of inactivity
Projects
None yet
Development

No branches or pull requests

3 participants