Problem/Motivation
Project Browser supports an undocumented allowed_projects configuration option, which is supposed to be a configurable filter of which projects a particular source can show. We should outright remove it before stable.
Why?
- It was added to support Drupal CMS at an earlier stage of development and was not well thought-out. At the time, individual sources were not configurable.
- The intended behavior was only ever implemented by the
recipessource. The config object theoretically will filter for any source, but in practice this does not happen and is not supported. So it's inconsistent. - This can be replaced by per-source configuration options if desired.
- It's fully superseded by the new
recommendedsource, which is a much more robust and sensible way to only show a specific set of projects.
Proposed resolution
Completely remove the config option in favor of the recommended source, which is functionally an allow-list unto itself.
Issue fork project_browser-3539470
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
phenaproximaComment #3
phenaproximaComment #4
phenaproximaComment #5
phenaproximaYeah, this is gonna need a backport.
Comment #7
phenaproximaComment #8
phenaproximaComment #9
phenaproximaAssigning to @tim.plunkett for review, commit, and backport. I think this change will make a lot of sense to him and he'll have a clear sense about whether the update path is sound.
Comment #10
phenaproximaDecided to postpone this on https://www.drupal.org/project/project_browser/issues/3305614, which I think would be a good place to add more generalized support for allow/deny lists in sources.
Comment #11
phenaproximaAfter thinking about how this would relate to #3305614: Allow a module to be blocked from display on Project Browser, I decided to change direction here.
Since it makes no logical sense to have both an allow list and a deny list operating on the same set of projects, I think what makes sense here is:
recommendedsource, which is functionally an allow-list unto itself.recipessource (and others) in #3305614: Allow a module to be blocked from display on Project Browser.Comment #12
phenaproximaComment #13
phenaproximaI can, however, see the use case for being able to filter another source through an allow-list. What I think might make sense here -- and would probably need to be a blocker for this issue -- is a whole new source, called
allow_listor similar, which decorates another source and filters its results through a configured allow-list. This wouldn't be too hard to do, and it would allow great flexibility for people who want to show "just some of the results from therecipessource", without needing to fully duplicate the information by usingrecommended.Comment #14
phenaproximaI've been thinking about this and here's what I propose.
There are two things we'd like to allow people to do:
As these are logically two different things, I think we should implement them as separate features.
The first thing is a reject list. We should implement a
hide_listconfiguration option in ProjectBrowserSourceBase and respect it by default increateResultsPage(). This allows site admins to specifically hide certain things from users all the time (a perfect use case for this would be hiding CTools, a module which should almost never be visible to Project Browser's target audience).The second thing is an allow list. This should probably be a totally new source, maybe called
subset, which could be a decorator that takes three configuration options:decorated_id(the plugin ID to decorate),decorated_options(configuration options to pass to the decorated plugin), andquery(the query array to pass to the decorated plugin, so as to guarantee that it only shows the stuff you want).Comment #15
chrisfromredfinI've tested !849 manually and can verify it doesn't interfere with any base functionality of PB. Will leave Tim to provide code review, if this is the path forward - seems clear it is from the IS but not from a re-read through the comments.
Comment #17
chrisfromredfinWorking on 2.1.x but has an issue in 2.0.x
Comment #19
phenaproximaComment #21
chrisfromredfinaaaaand scene.