Download & Extend
Issues

Change 'name' to 'id' on View entity

Posted by damiankloip on August 26, 2012 at 9:45pm
Project:Drupal core
Version:8.x-dev
Component:views.module
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:Configurables, Needs tests, VDC

Issue Summary

*Maybe* it would make more sense to change the view 'name' property to 'id'?

Don't forget about views_entity_info.

Login or register to post comments

Comments

#1

Posted by damiankloip on August 26, 2012 at 9:52pm
Status:active» postponed

#2

Posted by xjm on October 22, 2012 at 10:49pm
Project:Views» Drupal core
Version:8.x-3.x-dev» 8.x-dev
Component:Code» views.module

#3

Posted by damiankloip on January 2, 2013 at 2:51pm

Let's see how this gets on now.

AttachmentSizeStatusTest resultOperations
1757564-id.patch48.42 KBIdleFAILED: [[SimpleTest]]: [MySQL] 47,957 pass(es), 452 fail(s), and 84 exception(s).View details

#4

Posted by damiankloip on January 2, 2013 at 2:53pm
Status:postponed» needs review

#5

Posted by dawehner on January 2, 2013 at 3:25pm

This seems to be a patch which should be postponed until nearly the end of API freeze.

#6

Posted by damiankloip on January 2, 2013 at 3:29pm

I don't mind, just thought it was a postponed one that could be got out the way pretty easily. This has to be changed and the api around getting the id with id() and storing it in yaml won't change much :)

#7

Posted by System Message on January 2, 2013 at 8:31pm
Status:needs review» needs work

The last submitted patch, 1757564-id.patch, failed testing.

#8

Posted by tim.plunkett on January 2, 2013 at 8:44pm
Status:needs work» needs review

#3: 1757564-id.patch queued for re-testing.

#9

Posted by System Message on January 2, 2013 at 9:06pm
Status:needs review» needs work

The last submitted patch, 1757564-id.patch, failed testing.

#10

Posted by tim.plunkett on January 2, 2013 at 9:11pm
Status:needs work» needs review

#3: 1757564-id.patch queued for re-testing.

#11

Posted by System Message on January 2, 2013 at 10:13pm
Status:needs review» needs work

The last submitted patch, 1757564-id.patch, failed testing.

#12

Posted by damiankloip on January 3, 2013 at 9:02am
Status:needs work» needs review

Forgot to change the entity info in the annotation, it's always the small things.

AttachmentSizeStatusTest resultOperations
1757564-12.patch48.62 KBIdleFAILED: [[SimpleTest]]: [MySQL] 47,864 pass(es), 453 fail(s), and 86 exception(s).View details

#13

Posted by System Message on January 3, 2013 at 9:52am
Status:needs review» needs work

The last submitted patch, 1757564-12.patch, failed testing.

#14

Posted by damiankloip on January 14, 2013 at 11:36am
Status:needs work» needs review

Let's see how this gets on.

AttachmentSizeStatusTest resultOperations
1757564-14.patch106.84 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,159 pass(es), 201 fail(s), and 7 exception(s).View details

#15

Posted by System Message on January 14, 2013 at 12:12pm
Status:needs review» needs work

The last submitted patch, 1757564-14.patch, failed testing.

#16

Posted by damiankloip on January 14, 2013 at 1:11pm
Status:needs work» needs review

#14: 1757564-14.patch queued for re-testing.

#17

Posted by System Message on January 14, 2013 at 2:24pm
Status:needs review» needs work

The last submitted patch, 1757564-14.patch, failed testing.

#18

Posted by damiankloip on January 14, 2013 at 3:27pm
Status:needs work» needs review
AttachmentSizeStatusTest resultOperations
1757564-18.patch132.28 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,730 pass(es), 2 fail(s), and 0 exception(s).View details

#19

Posted by tim.plunkett on January 14, 2013 at 3:38pm

+++ b/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.phpundefined
@@ -165,7 +165,7 @@ public function uri() {
    * Overrides Drupal\Core\Entity\EntityInterface::id().
    */
   public function id() {
-    return $this->get('name');
+    return $this->get('id');

We can remove this method now (which was part of the reason to switch)

+++ b/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.phpundefined
@@ -207,7 +207,7 @@ public function isEnabled() {
   public function getHumanName() {

We should make a separate issue to make this just View::label()

+++ b/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.phpundefined
@@ -207,7 +207,7 @@ public function isEnabled() {
+      $human_name = $this->get('id');

Let's try to use id() directly

#20

Posted by damiankloip on January 14, 2013 at 3:48pm

Thanks Tim. Yeah, That must have been a missed ->get('id') there. Also removed the id() method.

AttachmentSizeStatusTest resultOperations
1757564-20.patch132.38 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,755 pass(es), 2 fail(s), and 0 exception(s).View details
interdiff.txt887 bytesIgnored: Check issue status.NoneNone

#21

Posted by tim.plunkett on January 14, 2013 at 4:02pm

Created #1888390: [Change notice] Change 'human_name' to 'label' and replace View::getHumanName() with View::label()

#22

Posted by dawehner on January 14, 2013 at 4:17pm

Let's fix the two most horrible lines in views.

Every other single line of the patch looked perfect!

AttachmentSizeStatusTest resultOperations
interdiff.txt1.21 KBIgnored: Check issue status.NoneNone
drupal-1757564-22.patch132.96 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,711 pass(es), 2 fail(s), and 0 exception(s).View details

#23

Posted by System Message on January 14, 2013 at 5:05pm
Status:needs review» needs work

The last submitted patch, drupal-1757564-22.patch, failed testing.

#24

Posted by damiankloip on January 14, 2013 at 5:20pm
Status:needs work» needs review

We just missed the cloning stuff exposed by the default views test.

AttachmentSizeStatusTest resultOperations
1757564-24.patch135 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1757564-24.patch. Unable to apply patch. See the log in the details link for more information.View details
interdiff.txt2.04 KBIgnored: Check issue status.NoneNone

#25

Posted by dawehner on January 14, 2013 at 9:58pm
Status:needs review» reviewed & tested by the community

Great!

#26

Posted by tim.plunkett on January 15, 2013 at 12:28am
Title:Change 'name' to 'id' on config/ViewStorage» Change 'name' to 'id' on View entity

#27

Posted by catch on January 17, 2013 at 9:14pm

#24: 1757564-24.patch queued for re-testing.

#28

Posted by System Message on January 18, 2013 at 12:57am
Status:reviewed & tested by the community» needs work

The last submitted patch, 1757564-24.patch, failed testing.

#29

Posted by tim.plunkett on January 18, 2013 at 1:03am
Status:needs work» needs review

Rerolled.

AttachmentSizeStatusTest resultOperations
vdc-1757564-29.patch135.01 KBIdleFAILED: [[SimpleTest]]: [MySQL] 50,599 pass(es), 1 fail(s), and 0 exception(s).View details

#30

Posted by System Message on January 18, 2013 at 5:40am
Status:needs review» needs work

The last submitted patch, vdc-1757564-29.patch, failed testing.

#31

Posted by damiankloip on January 18, 2013 at 7:35am
Status:needs work» needs review

re roll didn't have the super new views serializer support in :)

AttachmentSizeStatusTest resultOperations
1757564-31.patch136.31 KBIdlePASSED: [[SimpleTest]]: [MySQL] 50,624 pass(es).View details
interdiff.txt1.3 KBIgnored: Check issue status.NoneNone

#32

Posted by dawehner on January 18, 2013 at 7:37am

There we go.

AttachmentSizeStatusTest resultOperations
drupal-1757564-31.patch136.31 KBIdlePASSED: [[SimpleTest]]: [MySQL] 50,613 pass(es).View details

#33

Posted by dawehner on January 18, 2013 at 7:37am
Status:needs review» reviewed & tested by the community

Two times the exact same patch.

#34

Posted by tim.plunkett on January 19, 2013 at 4:48am
Issue tags:+Configurables

Tagging, since this brings us in line with the other config entities.

#35

Posted by webchick on January 19, 2013 at 5:11am
Title:Change 'name' to 'id' on View entity» Change notice: Change 'name' to 'id' on View entity
Status:reviewed & tested by the community» active
Issue tags:+Needs change notification

Yay for moar consistency.

Committed and pushed to 8.x. Thanks!

This'll need a tiny change notice.

#36

Posted by dawehner on January 19, 2013 at 7:49am

The change notice: http://drupal.org/node/1893002

#37

Posted by olli on January 30, 2013 at 6:13am

Change notice:

-  $view_name = $view->name;
+  $view_name = $view->storage->id();

Patch:
+++ b/core/modules/views/lib/Drupal/views/Plugin/Core/Entity/View.php
@@ -48,11 +48,11 @@ class View extends ConfigEntityBase implements ViewStorageInterface {
   /**
-   * The name of the view.
+   * The unique ID of the view.
    *
    * @var string
    */
-  public $name = NULL;
+  public $id = NULL;

Can we now use $view->id() instead?

#38

Posted by olli on January 30, 2013 at 6:25am
Status:active» needs work

+++ b/core/modules/views/includes/ajax.inc
@@ -208,7 +208,7 @@ function views_ajax_command_replace_title($title) {
-    'siteName' => config('system.site')->get('name'),
+    'siteName' => config('system.site')->id(),

+++ b/core/modules/views/views.theme.inc
@@ -877,7 +877,7 @@ function template_preprocess_views_view_rss(&$vars) {
   if ($view->display_handler->getOption('sitename_title')) {
-    $title = $config->get('name');
+    $title = $config->id();
     if ($slogan = $config->get('slogan')) {
       $title .= ' - ' . $slogan;

Oops.

EDIT: The first one was already fixed in #1896990: Cannot edit a View's title from the UI.

#39

Posted by dawehner on January 30, 2013 at 6:54am
Status:needs work» needs review

There we go.

AttachmentSizeStatusTest resultOperations
drupal-1757564-39.patch581 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 48,681 pass(es).View details

#40

Posted by olli on January 30, 2013 at 7:10am
Status:needs review» reviewed & tested by the community

Looks good to me and /rss.xml works again.

#41

Posted by webchick on January 30, 2013 at 7:10am
Status:reviewed & tested by the community» needs work
Issue tags:+Needs tests

Why did tests not break?

#42

Posted by damiankloip on January 30, 2013 at 7:43am

Should this be moved into a follow up? Otherwise we have a change notice issue that needs tests, that's weird.

#43

Posted by dawehner on January 30, 2013 at 6:46pm

Yeah webchick is totally right there are certain pieces of code which does not have test coverage yet.
Working on the follw up: #1903410: Wrong config key is used in template_preprocess_views_view_rss

#44

Posted by dawehner on February 12, 2013 at 9:09pm
Status:needs work» fixed

So we can mark this as fixed, as we have a bugfix for the patch.

#45

Posted by System Message on February 26, 2013 at 9:10pm
Status:fixed» closed (fixed)

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

#46

Posted by Cottser on March 9, 2013 at 4:40am
Title:Change notice: Change 'name' to 'id' on View entity» Change 'name' to 'id' on View entity
Issue tags:-Needs change notification

Updating title and tags now that we have a change notice.

nobody click here