Issue #3348399 #3348407 #3353443 #3230125: Release SOLR based unified search with content tagging and profile refactor #3480
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for contributing towards Open Social! A maintainer from the @goalgorilla/maintainers group might not review all changes from all teams/contributors. Please don't be discouraged if it takes a while. In the meantime, we have some automated checks running and it might be that you will see our comments with some tips or requests to speed up the review process. 😊 |
Kingdutch
force-pushed
the
issue/3348399-3348407-3353443-solr-unified-search-with-content-tagging
branch
from
237b2c0
to
2167f9d
Compare
Kingdutch
force-pushed
the
issue/3348399-3348407-3353443-solr-unified-search-with-content-tagging
branch
from
2167f9d
to
90a28dc
Compare
Kingdutch
added
team: bananas
type: refactoring
17 tasks
This includes the visibility checks in the different kinds of search. This ensures access is properly tested and also tests the views filter out the right content from the search indices.
This removes the three specific indices and copies all of their contents to the social_all index. The views are adjusted to use the new index but filter with a datasource instead. This deduplicates the information stored in our search database. We now do need to update any code that touches the search and redirect them to the new index and the updated views. The config override for search and the group search view is moved into a new config/modify file. This allows us to update config on installation depending on what module/config are available. This can replace our config overrides since we can do the dynamic enable when new modules are enabled. (Config overrides were previously needed since features would revert their config on deploy, but this hasn't happened since 8.0)
We temporarily remove adding the social_tagging fields to search so that we can test our other search changes. The field will be re-added when we refactor the social_tagging module itself and move to using the Field API.
The test was added in #3381 but we are now testing for random sort order which is undesired. Instead we rewrite the test to have a reliable sort order. We can also remove custom steps since there are labels available that we can click.
The views allow users to select sorting options. However, in case a sorting option is chosen which results in a tie, then the view doesn't apply any tie-breaker. This commit adds a non-exposed `created` and `title` sorting for the different search types so that items with the same relevance have a consistent and predictable sorting order. Given that users don't have an indexed label we only use the `created` as a fallback in that case.
This commit introduces an EntityViewBlock which allows rendering an entity for the current context (e.g. the current route) for a specific view mode. This is useful if we want to display a certain field (like tags) in a separate block on the page rather than in the main content area. To accomodate this a new EntityRouteContext is created which allows getting any entity from a route (e.g. rather than only the node entity or a group entity) which allows blocks like these to be more generic (making them easier to manage). We copy the code for this from the Entity Route Context module rather than using the module. This is because the module itself will provide additional contexts for all the installed entity types. Open Social has a lot of entity types and a large part of those also already provide their own contexts, so this would cause a large pollution of the possible contexts with a lot of duplicates which would hinder maintenance. The copied code is small enough (a single service) that we feel it's worth it to duplicate this.
This commit looks like it does a whole lot but the important part is that it adds a bunch of fields and view modes to let Drupal control tagging. We create the fields for all the content types that support them. The new service uses the Field API to manage the visibility rather than relying on alter hooks, this should be more performant. An added benefit is that changes to the configuration from outside of Drupal are reflected in the form. Tests are updated to test the new system
We're already using the facet module in some Open Social extensions so it's not a new burden for some of our sites. The facet module allows us to create a search filter that can reduce the number of results by adding new filters. It helps us create a complex UX where a user is prevented from making choices that won't refine their search results. The facet module also provides a hierharchical widget that matches wat we want for the content tagging.
We previously removed the duplicate split/nosplit templates. This adjusts the follow formatter to actually include the classes needed for the CSS and JS to work.
The group update adds the variation cache module however code in the group module is triggered during other updates which relies on that module, causing a fatal error. For background see https://www.drupal.org/project/group/issues/3134690 https://www.drupal.org/project/group/issues/3135757 https://www.drupal.org/project/drupal/issues/3150512 See 0adf6c0
This moves the widget and formatter functionality from social_tagging into a social_tag_split module. This allows it to also be reused by other functionality like the social_profile's profile tagging, without having to enable the social_tagging module which comes with content tagging.
This commit removes the profile tag settings and brings the feature in line with how content tagging works. We always enable the tag splitting because the no-split scenario can be emulated by only creating a single category. The access management can now be performed through the generic field settings and no longer requires special settings. The `use_category_parent` existed to allow searching for profiles with “any tag in category” which doesn’t require selecting a special value during editing but can be done during search using the facets widget.
In a9f41db we ensured that search queries required the `list user` permission so that search can not be used to enumerate users. This was needed because otherwise users might show up on the "All" search page. The user search still had a custom permission. However we don't actually expose a configuration page for that permission. So we change the view to use the permission that is actually needed in the underlying data system and we remove the special purpose permission.
We had initially deprecated the SocialProfileTagSplitWidget in 12.0.0 for removal in 13.0.0. However, it relies on the SocialProfileTagService for which we have already migrated and cleaned up the config, so in practice those systems already don't work. I think it's better to remove the code and have a hard-break than keep the code around in a non-functioning matter creating subtle bugs for people. I also don't want anyone to accidentally build on or extend the old system. Unfortunately it is not possible to deprecate these things in 11.x already because the replacement can't be introduced until 12.0.0. This makes the break a bit harder but we'll write an update guide to help out developers.
The required setting for the profile tag field wasn't propagated to the subfields that were being created.
This has been quite the challenge. There is code that's unaware of the existence of profiles, but that inadvertently triggers code that requires profiles. For example the `ginvite` module needs to send notifications when a user is created and does this in `hook_user_insert`. However this includes the display name of the account to address the user. We want to do this with their name if they've filled out this information on the registration page. We use module_implements_alter to change the order in which hooks fire to make sure the profile and social_profile hooks are always run first. This ensures whenever a user is saved, a profile will exist (together with the profile patch added earlier) and makes sure the user's name is properly generated if available. A backtrace is added to logging because figuring all this out without it was a pain.
We already had this styling in our FollowTag formatting, but it's actually a universal design pattern for tags in Open Social that they're displayed as small pills. To accomplish this we add the classes in our TagSplitFormatter so that themes can use this. This is accompanied by a field file change in socialbase that ensures the main label for the profile tag field is not shown but the individual labels of the subfields are rendered as labels instead.
When the role was introduced we installed new sites with "Verified user" capitalization but the update introduced it with "Verified User". In case the roles are used in labels that are clicked in tests, these differences in capitalization are problematic, and it also just doesn't look the way we intend for a large part of our customerbase. Our designer gave the following answer: "Based on our microcopy approach (Slite/Humans/Product language) we don’t use Title Case, which means that the right option is ‘Verified user’." This commit updates existing sites and pulls them in line with the correct spelling already used in new sites.
It's a bit weird to add a patch for something we manage. However we have a chicken/egg problem. We can't add the new twig file to socialbase because it'll break the existing profile tagging functionality. However, we require it to properly display the new profile tagging display structure and pass our tests. We include the patch until the profile refactor is released, then we can tag a new socialbase version with the fix and update within Open Social while removing the patch.
Profile tagging was something that was allowed for content managers. So in our upgrade path the content managers get the permission by default. This breaks our test which expects the content manager not to have access to any fields and thus see an access denied page.
The subfield settings upgrade path did not take the scenario where the social_profile_fields module was disabled. We implement the fallback to act as if the module was disabled and use the default address field behaviour.
Kingdutch
force-pushed
the
issue/3348399-3348407-3353443-solr-unified-search-with-content-tagging
branch
from
b4f340d
to
2b64020
Compare
18 tasks
ronaldtebrake
modified the milestones:
13.0.0-alpha2,
13.0.0-alpha3,
13.0.0-alpha4,
13.0.0-alpha5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem / Solution
This brings together about 2 years of work which has been merged into a single branch through separate PRs but needs to be delivered to the product as a single unit.
The suggestion is that individual PRs should be reviewed and no changes are made in this PR.
Included:
#3354
#3384
#3443
#2478
Points of attention
Issue tracker
Theme issue tracker
[Required if applicable] Paste a link to the drupal.org theme issue queue item, either from socialbase or socialblue. If any other issue trackers were used, include links to those too.
How to test
Definition of done
Before merge
After merge
Screenshots
Release notes
Change Record
Translations