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

Added multitag support #542

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Added multitag support #542

wants to merge 36 commits into from

Conversation

sarjona
Copy link
Contributor

@sarjona sarjona commented May 6, 2016

By default the code of this module supports more than one tag separated by comma (bpd_tag=tag1, tag2), but it's not possible to select more than one from the filtering page. This patch lets select more than one tags when filtering:

  • Reviewed styles to show selected tags.
  • Fixed bug with bp-docs to let AND filter. By default filtering is done with OR clausure.
  • Added option to select more than one tag from filtering page.
  • Added link to let users choose if the filter has to include all the tags (AND) or only some of them (OR). For this part, at the moment, it's necessary to add the following in the .po file:
msgid "<strong><a href=\"%s\" title=\"View Docs with some of the selected tags\">View Docs with some of the selected tags</a></strong>"
msgstr ""

msgid "<strong><a href=\"%s\" title=\"View Docs with all selected tags\">View Docs with all selected tags</a></strong>"
msgstr ""

dcavins added 20 commits March 22, 2016 11:22
Store the calculated array of protected doc IDs in the
`BP_Docs_Access_Query` singleton class to avoid calculating the list
more than once.
Adds a new taxonomy to store the comment access setting for each doc.
This will allow us to protect comments that should be hidden from the
user.
Add comment-specific code to BP_Docs_Access_Query so that we can filter
comments based on doc settings.
Two major components:
* Filter WP_Comment_Query so that comments on docs are handled
according to the doc’s privacy settings.
* Protect activity stream items that are created by commenting on a doc.
* Rather than storing this data in the singleton, move to WP cache with
dreams of moving to a persistent cache in the future.
* Add tests showing that the access protection queries are not
duplicated.
Store the calculated array of protected doc IDs in the
`BP_Docs_Access_Query` singleton class to avoid calculating the list
more than once.
Adds a new taxonomy to store the comment access setting for each doc.
This will allow us to protect comments that should be hidden from the
user.
Add comment-specific code to BP_Docs_Access_Query so that we can filter
comments based on doc settings.
Two major components:
* Filter WP_Comment_Query so that comments on docs are handled
according to the doc’s privacy settings.
* Protect activity stream items that are created by commenting on a doc.
* Rather than storing this data in the singleton, move to WP cache with
dreams of moving to a persistent cache in the future.
* Add tests showing that the access protection queries are not
duplicated.
Sometimes, we need to handle access queries for multiple users on a
page load. The existing singleton structure returned the singleton
calculated for the first user every time.
If a user shouldn’t be able to see the activity item, don’t send them
an email about it either. Adds a filter for the “all email” action and
the action that adds activities to the digest queue.
New tests:
* Non-logged-in users shouldn’t see logged-in only items.
* No one but the author should see creator-only items.
* Non-group members shouldn’t see group-member-only items.
… activity-stream-exclusions

# Conflicts:
#	includes/activity.php
#	tests/test-bp-docs.php
Introduce a `last_changed` incrementor so that restricted doc IDs,
comment doc IDs, and comment IDs stay fresh.
Send `bp_docs_update_doc_access()` the right values to work with.
Add test for `get_doc_ids()`. Test protections
for docs with read access limited to
group admins and mods.
Add tests for `get_restricted_comment_doc_ids()` and
`get_comment_ids()`. Test loggedin, creator-only, group members, and
group admins and mods protections for docs with limited comment read
access and their child comments.
Earlier tests checked that users were prevented from accessing certain
docs. These new tests check that user are not prevented from  accessing
docs to which they should have access.
Mark the changes related to comment and activity item protection as
going in at version 2.0, not 1.9.1.
@sarjona
Copy link
Contributor Author

sarjona commented May 6, 2016

multitag_bpdocs

boonebgorges and others added 8 commits May 6, 2016 13:48
`get_post()` falls back on the `$post` global when the `$post_id` passed to it
is empty, so we should be sure not to call the function if the doc ID provided
by the activity item is empty.

Props to @bosshy for diagnosing the issue.

Fixes #536.
This causes problems during unit tests.
@boonebgorges
Copy link
Owner

Thank you so much for the pull request! I love the functionality.

A couple questions. First, there are some merge conflicts that are arising, primarily because you are working off of master rather than the latest release branch. (Sorry for not having better documentation of this.) If it's not too much trouble, could you regenerate the PR using the 2.0.x branch? I think the following should do it:

$ git checkout WIP-multitag
$ git checkout -b WIP-multitag-backup
$ git branch -D WIP-multitag
$ git checkout -b WIP-multitag upstream/2.0.x # replace "upstream" with whatever you've called my repo
$ git cherry-pick 5db0b56
$ git push --force origin WIP-multitag

Github should recognize that you're using the same branch name, and update this PR. (If you can't do all of this, we can continue to work off of the current PR.)

On the substance of the PR:

  • I think the text "You are viewing docs with the following tags" should be modified according to the current boolean. So the default view would be "You are viewing docs with one or more of the following tags", and when the bool=and, it'd be something like "You are viewing docs with each of the following tags". So it'd work kinda like your contextual link "View docs with all selected tags", which I really like.
  • If you don't mind, could you modify your spacing to match WP formatting standards? https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#space-usage

Thanks!

@sarjona
Copy link
Contributor Author

sarjona commented May 9, 2016

Hi!
It's great that you like this functionality! We have recreated the WIP-multitag branch from the last content of 2.0.x branch, we have adapted the strings with your proposal and we have modified the spacing mistakes. So, the WIP-multitag now has to be correct... Please, tell us if you can merge from this PR or if you need we make a new one.
Kind regards!

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.

4 participants