Problem/Motivation

#2433281: Move Role Constants on to a Class/Interface deprecates DRUPAL_ANONYMOUS_RID and DRUPAL_AUTHENTICATED_RID for removal some time before the 9.0.0 release.

Also the whole point of a file named 'boostrap.inc' is that you shouldn't have dependencies, such as AccountInterface.

Proposed resolution

Remove them.

Remaining tasks

Add change record.

User interface changes

API changes

Removes constants from bootstrap.inc.

Data model changes

Original issue:

#935062: Change role id to machine name changed DRUPAL_ANONYMOUS_RID and DRUPAL_AUTHENTICATED_RID to *_ROLE and the value of the constants are 'anonymous_user' and 'authenticated_user'. Since the purpose of the constants was increased readability (over 0 and 1), this purpose is now obviously defeated.
Ergo, kill them!

CommentFileSizeAuthor
#12 remove-1500140-12.patch1.05 KBhussainweb
#12 interdiff-6-12.txt1.05 KBhussainweb
#6 change_rid_to_role-1500140-5.patch897 bytesnitesh pawar
#3 change_rid_to_role-1500140-3.patch1003 bytesnitesh pawar

Comments

blueminds’s picture

blueminds commented
Issue summary: View changes

Why is this postponed? Still relevant?

mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Postponed » Active

const DRUPAL_AUTHENTICATED_RID = 'authenticated'; is still in bootstrap.inc

Postponed doesn't make much sense. The issue summary is confusing too.

nitesh pawar’s picture

nitesh pawar commented
Status: Active » Needs review
StatusFileSize
new change_rid_to_role-1500140-3.patch1003 bytes

Status: Needs review » Needs work

The last submitted patch, 3: change_rid_to_role-1500140-3.patch, failed testing.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented 
  1. +++ b/core/includes/bootstrap.inc
@@ -1,4 +1,4 @@
-<?php
+?php

    This shouldn't be edited. 

  2. +++ b/core/includes/bootstrap.inc
@@ -55,7 +55,7 @@
-const DRUPAL_ANONYMOUS_RID = AccountInterface::ANONYMOUS_ROLE;
+const DRUPAL_ANONYMOUS_ROLE = AccountInterface::ANONYMOUS_ROLE;

@@ -64,7 +64,7 @@
-const DRUPAL_AUTHENTICATED_RID = AccountInterface::AUTHENTICATED_ROLE;
+const DRUPAL_AUTHENTICATED_ROLE = AccountInterface::AUTHENTICATED_ROLE;

    Looks like patch was generated backwards?

nitesh pawar’s picture

nitesh pawar commented
Status: Needs work » Needs review
StatusFileSize
new change_rid_to_role-1500140-5.patch897 bytes

Pls Ignore this patch..

nitesh pawar’s picture

nitesh pawar commented
Status: Needs review » Active
AbdealiJK’s picture

AbdealiJK commented

I notice that now *_ROLE is deprecated in 8.0.x and for now it is set at AccountInterface::ANONYMOUS_ROLE and AccountInterface::AUTHENTICATED_ROLE.
So, is this task still valid ?

Currently, AccountInterface::AUTHENTICATED_ROLE is set as "authenticated" and AccountInterface::ANONYMOUS_ROLE as "anonymous"

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Status: Active » Needs work
Issue tags: +@deprecated, +Needs change record
Related issues: +#2433281: Move Role Constants on to a Class/Interface, +#935062: Change role id to machine name

Really giving me a headache in terms of autoloading on behalf of Drupal.

I think this could happen before 8.1.x, but I'm not sure if it's an API change.

I found this change record which tells you to use these constants, even though it's clear they should go: https://www.drupal.org/node/1619504

Edited that change record in light of #2433281: Move Role Constants on to a Class/Interface

Here's a review of #6 even though it says to ignore. :-)

The test seems to pass after simply removing the constants, so core seems to no longer be using these constants.

+++ b/core/includes/bootstrap.inc
@@ -64,7 +64,7 @@
  *   Use Drupal\Core\Session\AccountInterface::AUTHENTICATED_ROLE or
  *   \Drupal\user\RoleInterface::AUTHENTICATED_ID instead.
  */
-const DRUPAL_AUTHENTICATED_RID = AccountInterface::AUTHENTICATED_ROLE;
+const DRUPAL_AUTHENTICATED_ROLE = AccountInterface::AUTHENTICATED_ROLE;

Also remove the docblocks, please.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.0.x-dev » 8.1.x-dev
hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-6-12.txt1.05 KB
new remove-1500140-12.patch1.05 KB

I think the patch in #6 shows that we can remove the constants. I am removing them now.

Also, this seems like a BC break. I am not sure if this can go to D8.1. I will let the maintainers decide that.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Version: 8.1.x-dev » 9.x-dev

Hey, we cannot remove global constants, they might be used in existing contrib or custom modules and themes. They ar deprecated to be removed in 9.0.x.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Closed (duplicate)

Duplicate of #2716163: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 9 branch.

Version: 9.x-dev » 9.0.x-dev

The 9.0.x branch will open for development soon, and the placeholder 9.x branch should no longer be used. Only issues that require a new major version should be filed against 9.0.x (for example, removing deprecated code or updating dependency major versions). New developments and disruptive changes that are allowed in a minor version should be filed against 8.9.x, and significant new features will be moved to 9.1.x at committer discretion. For more information see the Allowed changes during the Drupal 8 and 9 release cycles and the Drupal 9.0.0 release plan.