Fix Bookmarks view is missing a display

This patch removes the controller, route, and link task (contextual link, user profile tab). These are all replaced by the updated View configuration.

Accidentally included a project specific change in #2. Re-rolled.

Patch #3 doesn't apply the same permission rules. Need to specify a views permission handler that allows view access for owners and those with "access others bookmarks" permissions.

Also needs to add the title on the default display.

 display:
   default:
     ...
     display_options:
+      title: Bookmarks

Updated patch to include default view default display title.

After some digging, to replace the access checks removed, this patch's view is going to need the following permissions checks:

• access others bookmarks
• Current user is viewing own bookmarks list

I was able to solve the views access constraint noted in #7.

It still needs a hook update for the views changes. For now, I pushed this patch and MR up to get community feedback.

Anyone applying this patch wanting to adopt the views config changes may run the following:

cat web/modules/contrib/bookmarks/config/optional/views.view.bookmarks.yml | lando drush config:set --input-format=yaml views.view.bookmarks ? -

I guess I need to update the target branch. Considering there will be a hook_update_N(), this should bump minor version from 1.0.x to 1.1.x, right?

I'd also like to rename the Views Access plugin.

Fixes a bug with Bookmarks tab not showing.

We've adopted patch #12 to a production project, I am happy to place this to RTBC; however, I am chewing on whether this needs a hook update for the veiws.view.bookmarks changes and whether this would need a major, minor, or patch version bump.

Obviously, the fix is adopted for any new install. But, for this fix to apply to existing installations would override their existing Bookmarks view--this could be considered a breaking change requiring a major version bump.

The way we applied the change was to simply import the single updated configuration from this module, and then used git to help us track which changes we wanted to keep:

cat web/modules/contrib/bookmarks/config/optional/views.view.bookmarks.yml | drush config:set --input-format=yaml views.view.bookmarks ? -

I imagine we could just bump the patch version and include this instruction in the release notes.

So, the above boils down to, either we:

1. Include a hook update and bump the major version.
2. Include the single config import instructions in the release notes and bump the patch version.

Opinions welcome.

As we consider this, these resources may help with development:

• function hook_update_N
• Don’t go making major version changes
• 📌 Document how contrib hook_update_N() should be numbered now that modules can be compatible with multiple major branches and versioned semantically Fixed

Leaving this one last note here for if we do decide to go with a hook update.

function bookmarks_update_VERSION() {
  $config_factory = \Drupal::configFactory();
  $view = $config_factory->getEditable('views.view.bookmarks');

  if ($view->get('display.default.display_options.access.type') != 'bookmarks_view_access') {
    $view->set('display.default.display_options.access.type', 'bookmarks_view_access');
    $view->set('display.default.display_options.title', 'Bookmarks');
    ...
    $view->save();
  }
}

This also solves a previously hidden views permissions issue which prevented non-admin users from seeing the links and title fields reported at 🐛 Fix bookmark link not shown for non-admin users Fixed #8 🐛 Fix bookmark link not shown for non-admin users Fixed .

Still awaiting community feedback on options 1 or 2 noted in #13. 🐛 Fix Bookmarks view is missing a display Needs review

If it really is a breaking change, that warrants a major bump. But, I’m beginning to think it is not a breaking change. The controller and route were in place because the view had not controlled that without a page display. With this change, it does. It also shifts access control to the view with a new view access plugin. So, all the update is confined to the one view.

With a hook update, we could override any customizations they may have made to the view. Without the hook update, if they don’t run the view update config:set, they won’t get the changes and it’ll break.

I’m of the opinion that if they customize a module-provided view, any config changes upon update is there’s to deal with.

That said, I’m for a minor update to 1.1.0 and a hook update to close out this ticket.