-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add support for allow_seen in Recommendation requests #87
Conversation
4b6631a
to
cfd1929
Compare
/** | ||
* Allow items, that the user has already "seen" | ||
* | ||
* By default user won't see any items, that he or she has visitted (and we have recorded DetailView interaction.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it has visited
* If you want to circumvent this, and get recommendations including the ones, that the user has already visitted, | ||
* you can set the "seen" allowance here. | ||
*/ | ||
public function setAllowSeen(bool $seen = true): self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing - we should either has setter setAllowSeen(bool)
with required parameter, or two methods to set it to true and false (allowSeen()
and eg. disallowSeen()
). For simplicity I'd go for the first variant, but for sure not mix those two approaches.
@@ -242,6 +258,10 @@ protected function getCommandParameters(): array | |||
$parameters['model_name'] = $this->modelName; | |||
} | |||
|
|||
if ($this->allowSeen) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather strict comparison (eg. !== false
) instead of this truthy condition.
cfd1929
to
5d8b09f
Compare
>= 7.46.0
By default, "seen" is any item, that the user has DetailView'ed. This however, can be changed for any given model, so "seen" can mean different things in
default
andgamma
model for any particular client.