Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

FileSize
10.91 KB

Just some work :)

xjm’s picture

+++ b/core/modules/forum/forum.views.incundefined
@@ -0,0 +1,238 @@
+  $data['forum_index']['vid'] = array(j ¶

Oops. :)

+++ b/core/modules/user/user.api.phpundefined
@@ -472,5 +472,27 @@ function hook_user_role_delete($role) {
 /**
+ * Allows you act when a permission is enabled for a user.
+ *
+ * Allows you act when a user role has been deleted. If your module stores
+ * references to roles, it's recommended that you implement this hook and delete
+ * existing instances of the deleted role in your module database tables.
+ *
+ * @param $permissions
+ *   An array of permissions.
+ */
+function hook_user_permissions_insert($permissions) {

Uh...?

dawehner’s picture

FileSize
6 KB

Ups.

xjm’s picture

Status: Active » Needs work

Since there's a partial patch.

larowlan’s picture

Yes, that would mean a lot of the /forum pages could be default views.
I assume this can go in after Feb?

xjm’s picture

Yep, these integrations are okay after Feb. 18.

andypost’s picture

marked as duplicate #309001: Views integration

podarok’s picture

Status: Needs work » Needs review

status

xjm’s picture

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

Actually still NW I think.

Anonymous’s picture

May I be... OK, I'm *going* to be as bold as to ask the theory behind keeping Forum in core. Is it because there's many hanging off it already so if we removed it it would cause issues? We removed blog - that's a relief - but after looking at the forum code I really don't know why it's there - surely it's a contrib thing as we need to focus more on making the framework work?

Apologies if just jumping in and saying but I'm getting old and time is running out for me.

Ta

s

damiankloip’s picture

This issue is just about adding views integration for forum, I think whether it should be removed from core or not is a different issue altogether :)

I agree though, and would be pretty happy if I awoke one morning to find forum module gone...

Anonymous’s picture

damiankloip’s picture

+++ b/core/modules/forum/forum.views.incundefined
@@ -0,0 +1,238 @@
+    'title' => t('NID'),
+    'help' => t('The NID of the forum index entry.'),

We prob need to chnage these labels.

+++ b/core/modules/forum/forum.views.incundefined
@@ -0,0 +1,238 @@
+    'title' => t('VID'),
+    'help' => t('The VID of the forum index entry.'),

Likewise.

xjm’s picture

Yep: Node ID, Vocabulary ID.

dawehner’s picture

Metadata is probably as bad as "Views properties" :) What about "Edit Name / other basic"?

dawehner’s picture

Status: Needs work » Needs review
FileSize
11.98 KB

Sometimes you are just in the wrong issue :)

Posting a basic test.

andypost’s picture

+++ b/core/modules/forum/forum.views.incundefined
@@ -0,0 +1,160 @@
+  $data['forum_index']['tid'] = array(
...
+    'argument' => array(
...
+    'filter' => array(
...
+    'relationship' => array(

No sort and field definition. Why?

+++ b/core/modules/forum/forum.views.incundefined
@@ -0,0 +1,160 @@
+  $data['forum_index']['comment_count'] = array(
...
+    'filter' => array(
+      'id' => 'numeric',
+    ),
+    'sort' => array(
+      'id' => 'standard',

Why this IDs are different?

damiankloip’s picture

Why this IDs are different?

Not sure exactly what you mean, but we have a numeric filter plugin, but for sorts, the standard is fine.

dawehner’s picture

FileSize
483 bytes
12.03 KB

The reason why I removed the field from there is that this might confuse users, as you don't have enough information available here in order to render a full field, so our approach of using entities doesn't work at all. The only thing we can support is to display the actual number, which does this patch.

podarok’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

#19 looks good and covered with tests
RTBC

xjm’s picture

Issue tags: -VDC

#19: drupal-1867642-19.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, drupal-1867642-19.patch, failed testing.

dawehner’s picture

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

#19: drupal-1867642-19.patch queued for re-testing.

xjm’s picture

Status: Needs review » Reviewed & tested by the community

Restoring status.

xjm’s picture

Issue tags: -Needs tests

And fixing tags. :)

xjm’s picture

#19: drupal-1867642-19.patch queued for re-testing.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

From IRC

08:04aalexpott:
timplunkett: just reviewing http://drupal.org/node/1867642… do you think that the tests should cover every field added by forum_views_data()?
08:21atimplunkett:
alexpott: i don't think we need one for EVERY field
08:21aalexpott:
timplunkett: ok… so would we get an error if one was declared incorrectly?
08:23atimplunkett:
alexpott: ah!
08:23atimplunkett:
alexpott: it should be added to HandlerAllTest
damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.7 KB
12.39 KB

Let's add this to HandlerAllTest, and fix up a couple of small things.

Also, changed human_name to label, but missed that off the interdiff.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Great additions!

alexpott’s picture

Title: Expose forum tables to views » Change notice: Expose forum tables to views
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 8.x-3.x-dev
Component: forum.module » Code
Status: Reviewed & tested by the community » Active
Issue tags: -Needs tests +Needs change record

Committed bc8686e and pushed to 8.x. Thanks!

Assigning to the views queue for change notice and possible backport...

ParisLiakos’s picture

i think this might have introduce a random failure? actually exception

Edit: Nvm this is a legit one, not random at all, sorry for noise

larowlan’s picture

Issue summary: View changes

Anyone know of an issue to use this for /forum /forum/x I can't find one with google/d.o/email/metas?

mgifford’s picture

Is this now a D8 Views issue or should it be a D7 Views issue now that it's committed to D8 Core?

jibran’s picture

Title: Change notice: Expose forum tables to views » Expose forum tables to views
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.0.x-dev
Component: Code » forum.module
Status: Active » Fixed
Issue tags: -Needs change record

RE: #32 #2207263: Try and build /forum and /forum/{tid} with views
4 years without a change notice I think we are good here. If someone wants to backport it feel free to create a new issue in views queue.

Status: Fixed » Closed (fixed)

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