Support from Acquia helps fund testing for Drupal Acquia logo

Comments

InternetDevels’s picture

Status: Active » Needs review
FileSize
1.36 KB

Patch attached.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Nice clean-up!

@@ -161,7 +161,8 @@ function language_menu() {
 function language_access_language_edit_or_delete($language) {
-  return !$language->locked && user_access('administer languages');
+  $account = Drupal::request()->attributes->get('_account');
+  return !$language->locked && $account->hasPermission('administer languages');

Suppose this function would be droped in wscci conversion for 'language_admin_edit_form' and 'language_admin_delete_form'

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/modules/language/lib/Drupal/language/LanguageAccessController.php
    @@ -22,7 +22,8 @@ public function access(EntityInterface $entity, $operation, $langcode = Language
    
    @@ -22,7 +22,8 @@ public function access(EntityInterface $entity, $operation, $langcode = Language
    +        $account = \Drupal::request()->attributes->get('_account');
    

    This method has the account passed to it directly.

andypost’s picture

+++ b/core/modules/language/language.module
@@ -161,7 +161,8 @@ function language_menu() {
 function language_access_language_edit_or_delete($language) {
...
+  $account = Drupal::request()->attributes->get('_account');

Should be Drupal::currentUser() according https://drupal.org/node/2032447

InternetDevels’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

Patch attached.

InternetDevels’s picture

andypost’s picture

  1. +++ b/core/modules/language/language.module
    @@ -149,7 +149,8 @@ function language_menu() {
    +  $account = Drupal::currentUser();
    +  return !$language->locked && $account->HasPermission('administer languages');
    

    Please do not use $account here. Drupal::currentUser()->hasPermission() works fine

  2. +++ b/core/modules/language/lib/Drupal/language/LanguageAccessController.php
    @@ -18,11 +18,12 @@ class LanguageAccessController extends EntityAccessController {
    +    $account = is_null($account) ? \Drupal::currentUser() : $account;
    

    This is a hack, but reasonable

andypost’s picture

Use this to replace 2 hack

$account = $this->prepareUser($account);
andypost’s picture

Status: Needs review » Needs work
InternetDevels’s picture

Status: Needs work » Needs review
FileSize
1.49 KB

New one.

andypost’s picture

Status: Needs review » Needs work
+++ b/core/modules/language/lib/Drupal/language/LanguageAccessController.php
@@ -18,11 +18,12 @@ class LanguageAccessController extends EntityAccessController {
+    $account = is_null($account) ? \Drupal::currentUser() : $account;

$account = $this->prepareUser($account);

InternetDevels’s picture

Status: Needs work » Needs review
FileSize
1.46 KB

Fixed issue above.

tim.plunkett’s picture

+++ b/core/modules/language/lib/Drupal/language/LanguageAccessController.php
@@ -18,11 +18,12 @@ class LanguageAccessController extends EntityAccessController {
   public function access(EntityInterface $entity, $operation, $langcode = Language::LANGCODE_DEFAULT, AccountInterface $account = NULL) {
+    $account = $this->prepareUser($account);
     switch ($operation) {
       case 'create':
       case 'update':
       case 'delete':
-        return !$entity->locked && user_access('administer languages');
+        return !$entity->locked && $account->hasPermission('administer languages');
         break;
     }
     return FALSE;

This should be overriding checkAccess() and checkCreateAccess() instead. Then you don't need the prepareUser call.

deimos’s picture

Thanks, corrected patch regarding to comment above.

andypost’s picture

andypost’s picture

+++ b/core/modules/language/language.module
@@ -135,7 +135,7 @@ function language_menu() {
+  return !$language->locked && Drupal::currentUser()->hasPermission('administer languages');

\Drupal according #2053489: Standardize on \Drupal throughout core

areke’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

This needs to be re-rolled because the patch no longer applies.

InternetDevels’s picture

Status: Needs work » Needs review
FileSize
567 bytes
1.3 KB
andypost’s picture

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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