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

Allow selecting forums in addition to Index and Everywhere #149

Merged
merged 22 commits into from
Jun 28, 2024

Conversation

iMattPro
Copy link
Contributor

No description provided.

@iMattPro iMattPro force-pushed the locations branch 2 times, most recently from eada9dc to 35a2ed9 Compare June 26, 2024 18:39
@iMattPro iMattPro requested a review from rxu June 26, 2024 19:49
iMattPro added 11 commits June 26, 2024 21:43
Signed-off-by: Matt Friedman <[email protected]>

Fix

Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
{% if ba.LOCATIONS is empty %}
{{ lang('BOARD_ANNOUNCEMENTS_EVERYWHERE') }}
{% else %}
{% set has_index = constant('\\phpbb\\boardannouncements\\ext::INDEX_ONLY') in ba.LOCATIONS %}
Copy link
Contributor

Choose a reason for hiding this comment

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

How does {% set ... in ... %} construction work? Couldn't find it in docs.

Copy link
Contributor Author

@iMattPro iMattPro Jun 27, 2024

Choose a reason for hiding this comment

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

set defines a new variable. https://twig.symfony.com/doc/2.x/tags/set.html
in is like in array https://twig.symfony.com/doc/2.x/templates.html#containment-operator

This is creating a boolean variable: true if index is in the array of locations, false if not

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I see what do they mean individually but just curious how would it work together. So it adds has_index element into ba.LOCATIONS array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it creates a new twig template variable called has_index that is true when ba.LOCATIONS contains -1 which is then used in the next few lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's same as doing:

$has_index = in_array(ext::INDEX_ONLY, $locations);

@rxu
Copy link
Contributor

rxu commented Jun 27, 2024

There's an issue with announcements dismission (probably intended but not sure, also don't know if it was the case before).
If you have an announcement available for everyone and dismimm it as a guest, then if you log in with the same browser, the announcement is also dismissed for registered user. And vice versa.

@rxu
Copy link
Contributor

rxu commented Jun 27, 2024

Same applies to registered users: if user1 has dismissed the announcement, it's gonna be dismissed for any registered user on the same device and browser.

@iMattPro
Copy link
Contributor Author

That's how it always worked. It uses a cookie and a database entry if you were logged in.

User1 dismisses, it's gone for them forever. Even if they log out it's still gone because of cookies.

If a guest dismisses it's gone for that user on that device as long as the cookie doesn't expire.

@rxu
Copy link
Contributor

rxu commented Jun 27, 2024

May be add user_id to cookie to distinct users dismission?

@iMattPro
Copy link
Contributor Author

The cookie is to handle ANONYMOUS users

@rxu
Copy link
Contributor

rxu commented Jun 27, 2024

Then it shouldn't affect registered users, should it? But it does. Or does registered users dismission uses both cookie and DB part? Or am I missing the whole point?

@iMattPro
Copy link
Contributor Author

if a user closes an announcement, it never even gets queried again for them. so the cookie only affects them after they log out. if they switch to another account the cookie will keep it closed, but in what use case do different people use the same forum on the same device?

Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
@iMattPro iMattPro merged commit 07a637b into phpbb-extensions:master Jun 28, 2024
34 checks passed
@iMattPro iMattPro deleted the locations branch June 28, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants