I think we should be near a point where we can do this now. Move all the default views currently provided by views into their respective core modules. I think this should also mean we can kill the load() method we are overriding on the ViewStorageController.

But...

We currently have generic default views tests, that just test all of them at once.

Here is an initial patch. See how we get on.

Files: 
CommentFileSizeAuthor
#9 1863512-8.patch2.73 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 52,199 pass(es), 1 fail(s), and 1 exception(s).
[ View ]
d8.move-default-views.patch2.68 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Comments

dawehner’s picture

It seems to make sense to do this step by step in every actual conversion issue (tracker/frontpage etc.) and just keep the other ones here, maybe?

damiankloip’s picture

Status:Active» Needs review

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, d8.move-default-views.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review

d8.move-default-views.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+VDC

The last submitted patch, d8.move-default-views.patch, failed testing.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/ViewStorageController.phpundefined
@@ -16,21 +16,6 @@
-   * Overrides Drupal\config\ConfigStorageController::load();
-   */
-  public function load(array $ids = NULL) {
-    $entities = parent::load($ids);
-
-    // Only return views for enabled modules.
-    return array_filter($entities, function ($entity) {
-      if (module_exists($entity->get('module'))) {
-        return TRUE;
-      }
-      return FALSE;
-    });
-  }

I'm wondering what happens if we had node installed, then views got enabled and then node get's removed again. The config files might not dissapear and then you end up with loading unwanted files?

damiankloip’s picture

Hmm, I guess at the moment the file will not be removed. Is there a configuration issue for this? Couldn't it look at the config files in it's own config directory and makes sure they are removed?

dawehner’s picture

Maybe we should ask heyrocker about that, but that's maybe/probably done by the config system itself?

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new2.73 KB
FAILED: [[SimpleTest]]: [MySQL] 52,199 pass(es), 1 fail(s), and 1 exception(s).
[ View ]

I'm talking with alexpott about this at DC London, it is definitely something the config system should handle but in fact doesn't atm.

Here is a re roll anyway.

Status:Needs review» Needs work

The last submitted patch, 1863512-8.patch, failed testing.

dawehner’s picture

I propose to move the actual used views, but keep the example views (like glossary) in views.

damiankloip’s picture

That could be a good compromise I think.

dawehner’s picture

Issue tags:+Novice

Fill that as a novice task.

c_lehel’s picture

Issue summary:View changes
Status:Needs work» Needs review

I've started to look at this task because of the 'Novice' tag. I think due to the age of the issue many things have changed, or even got resolved.
comments_recent was moved to comment module, archive+frontpage+glossary were moved to the node module, taxonomy_term to the taxonomy module. I couldn't find the tracker and backlinks views, they were possibly renamed.
The controller, which is modified in the patch [ViewStorageController] I think was also removed/implemented elsewhere/in different way.

damiankloip’s picture

Status:Needs review» Fixed

Indeed, thanks for bringing this back up!

None of these are left in views now, all have been moved to their respective modules.

Status:Fixed» Closed (fixed)

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