Problem/Motivation

Profiles are conditionally added to the user view page render array via a view, profiles. In a #pre_render callback, profile_views_add_title_pre_render(), the view title is set to the label of the corresponding profile type.

If the current user doesn't have view access, the profile data is not rendered. However, the title is still rendered.

Expected result: the title is not rendered for profiles that the current user lacks view access to.

Proposed resolution

One approach would be to edit the view to add validation criteria to the "Profile: Profile" contextual filter. If the user lacks access, hide the view.

Alternately, in profile_views_add_title_pre_render(), suppress the title if no rows returned.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 0001-Issue-2748745-Profile-title-should-not-appear-on-use.patch1.45 KBestoyausente
#11 profile-title-274874-11.patch717 bytesnedjo
#15 profile-title-274874-15.patch1.96 KBnedjo
#15 profile-title-274874-15-test-only.patch1.26 KBnedjo

Comments

nedjo created an issue. See original summary.

jalpesh’s picture

jalpesh commented

Are you talking about removing $view->setTitle($element['#title']); from below function?

function profile_views_add_title_pre_render($element) {
/** @var \Drupal\views\ViewExecutable $view */
if (isset($element['#title'])) {
$view = $element['view_build']['#view'];
$view->setTitle($element['#title']);
}

return $element;
}

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Are you talking about removing $view->setTitle($element['#title']); from below function?

We shouldn't just remove it, since that call is needed to set the view's title to the label of the profile type. But we could examine the view display and e.g. conditionally suppress the title or block access to the display.

estoyausente’s picture

estoyausente
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new 0001-Issue-2748745-Profile-title-should-not-appear-on-use.patch1.45 KB

It seems easy. I fixed as the proposal solution: adding permission check in profile contextual filter and added an extra check when title is being override.

Status: Needs review » Needs work

The last submitted patch, 4: 0001-Issue-2748745-Profile-title-should-not-appear-on-use.patch, failed testing.

The last submitted patch, 4: 0001-Issue-2748745-Profile-title-should-not-appear-on-use.patch, failed testing.

estoyausente’s picture

estoyausente
Primary language Spanish
commented
Status: Needs work » Needs review

I don't know why the patch doesn't pass the tess. I test it in simplytest.me and the module is installed correctly and the view is changed.

Any know why the patch doesn't pass the test?

This is the test restult:

exception: [Uncaught exception] Line 93 of core/lib/Drupal/Core/Config/Testing/ConfigSchemaChecker.php:
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for views.view.profiles with the following errors: views.view.profiles:display.default.display_options.arguments.type.validate_options.bundles variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Sequence in Drupal\Core\Config\Testing\ConfigSchemaChecker->onConfigSave() (line 93 of /var/www/html/core/lib/Drupal/Core/Config/Testing/ConfigSchemaChecker.php). Drupal\Core\Config\Testing\ConfigSchemaChecker->onConfigSave(Object, 'config.save', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('config.save', Object) (Line: 227)
Drupal\Core\Config\Config->save(1) (Line: 280)
Drupal\Core\Config\Entity\ConfigEntityStorage->doSave('profiles', Object) (Line: 392)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 259)
Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object) (Line: 358)
Drupal\Core\Entity\Entity->save() (Line: 637)
Drupal\Core\Config\Entity\ConfigEntityBase->save() (Line: 321)
Drupal\Core\Config\ConfigInstaller->createConfiguration('', Array) (Line: 125)
Drupal\Core\Config\ConfigInstaller->installDefaultConfig('module', 'profile') (Line: 75)
Drupal\Core\ProxyClass\Config\ConfigInstaller->installDefaultConfig('module', 'profile') (Line: 248)
Drupal\Core\Extension\ModuleInstaller->install(Array, 1) (Line: 83)
Drupal\Core\ProxyClass\Extension\ModuleInstaller->install(Array, 1) (Line: 897)
Drupal\simpletest\WebTestBase->installModulesFromClassProperty(Object) (Line: 561)
Drupal\simpletest\WebTestBase->setUp() (Line: 60)
Drupal\profile\Tests\ProfileTestBase->setUp() (Line: 1046)
Drupal\simpletest\TestBase->run(Array) (Line: 723)
simpletest_script_run_one_test('1', 'Drupal\profile\Tests\ProfileTypeMultipleTest') (Line: 59)
jalpesh’s picture

jalpesh commented

Yes, you are right. I am able to apply successfully on my local machine. may be because of patch name, don't know just a guess... :)

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Status: Needs review » Needs work

Thanks for the draft patch!

Testing, I tried:

  • After patching, log in as admin user (UID 1) and install Profile.
  • Create a user, Tester.
  • Add a text field to default profile type, Personal information.
  • Create a custom Profile type, Test profile type, adding a text field.
  • Edit admin (UID 1) user's Personal information and Test profile type profiles to add a value for each text field.
  • Assign Personal information: View any profile and View user information permissions to Authenticated user role.
  • Log out, then log in as user Tester.
  • Result: forwarded to own profile page and hit fatal error:

    Fatal error: Call to a member function setTitle() on a non-object in /home/dralo/www/sites/default/modules/profile/profile.module on line 286

  • Navigate to user 1's profile page (/user/1). Same fatal error.
nedjo’s picture

nedjo
he/him/his
Primary language English
commented 
+++ b/config/install/views.view.profiles.yml
@@ -243,7 +243,11 @@ display:
-          validate_options: { }
+          validate_options:
+            access: true
+            operation: view
+            multiple: 0
+            bundles: null

If we had a profile argument (which we don't), we could validate that a user has access to a profile. But a profile type isn't viewed, so we shouldn't add the view access validation.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new profile-title-274874-11.patch717 bytes

Attached patch adds an access test on the profile being rendered. If the current user doesn't have access to the profile, the result won't be rendered, so skip setting the title.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Thanks! Could we also get a test on this?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue tags: +Needs tests
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented 
+++ b/profile.module
@@ -296,8 +296,13 @@ function profile_preprocess_views_view(&$variables) {
+    $entity = reset($view->result)->_entity;
+    if ($entity->access('view')) {

Wouldn't the results be empty if the user didn't have view access, anyways?

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
StatusFileSize
new profile-title-274874-15-test-only.patch1.26 KB
new profile-title-274874-15.patch1.96 KB

Here's the same patch with tests.

Wouldn't the results be empty if the user didn't have view access, anyways?

AFAIK what happens is the results are loaded but not rendered because an entity access check is done prior to rendering.

The last submitted patch, 15: profile-title-274874-15-test-only.patch, failed testing.

The last submitted patch, 15: profile-title-274874-15-test-only.patch, failed testing.

  • mglaman committed f2c7e77 on 8.x-1.x authored by nedjo 
    Issue #2748745 by nedjo, estoyausente, mglaman: Profile title should not...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Fixed

Thanks, nedjo! Especially for failing patch w/ test then patch with test+fix :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.