Full views support, admin/content/feed powered by views

The default view for the 'feed-item' path seemed mangled to me when I use the patch (It shows 'Invalid' view type). Removing $view->display['page'] = $display; from line 124 seemed to resolve it.

Patch applied cleanly on latest dev using OS X from command line. I installed patched module on a D6.8 system that already had FeedAPI 1.5 running with a few test feeds as well as Feed Element Mapper. I had changed the default view so reverted in order to pick up the new Views stuff provided by the patch. I am pretty new to Relationships in Views but it appeared to work fine. The only thing I saw was the Parent Feed was also showing on the Feed-Item view until I went into Relationship settings and clicked "Require this relationship" - After I did that the Parent Feed no longer showed up in the list (which is the behavior I wanted). Otherwise it appears to be working very well.

Patched module demo here: http://d6.metanomy.org/feed-item and if you go deep into the pager you can see Flickr and YouTube being mapped that still work fine.

On the Admin View: The concept of providing "admin" functions via a View caught me by surprise. Perhaps because Views is not listed as a dependency for FeedAPI? It may not be clear how the permissions on this View are handled. Access was set to "unrestricted" in the View. If you set the "access all views" permission for anonymous they can see the view but not the fields that provide the admin function - a good thing. I guess my feeling is that this View, if it has "admin" in the title, should be set to access>>permission>>administer feedapi by default. Or this could be called Parent Feeds (or something) and not given a path to or name that has anything to do with "admin" - It is a helpful view for anyone who would like to see a tabular listing of parent feeds on the site, and the edit/delete column would only show for people with the right permissions?

Anyway, nice work.

R,
Coby

drewish: The patch seems to be perfect. Nice job! :)
I modified the following:
- only privilegised (administer feedapi) users can view the admin/content/feed page
- Above the commands, the column header was Delete, i modified it to Commands

I have only one question now to everyone: is it a problem if FeedAPI depends on Views?
If this patch will be committed, this dependency has to be introduced. :(

At the admin views, there is a filter:

'type' => array(
      'operator' => 'in',
      'value' => array(
        'feed' => 'feed',
      ),

This is not appropiate. The user can create unlimited feed content-types. I imagine it's very hard to filter to a serialized variable in the variable table.
Also, the simpletest tests are need to be updated because of the admin page change.

#5: I think the admin/content/feed view should stay in the admin section: It should show all feeds, regardless whether they are published or not.

#6: I see your concern, but a hard dependency (.info) on views doesn't seem necessary. With this patch, FeedAPI will still work fine, there's just less functionality without views. Which is ok, given the fact that only very specific sites run without views. (and - it doesn't take away from FeedAPI given the fact that admin/content/feed is broken on sites with many feed items at the moment).

#7: Looks like the type array needs to be built dynamically. Which means we'll have to flush the views cache every time we enable/disable feedapi on a content type. This will mean some views related code in feedapi.module. A toggle in feedapi_content_type_submit() could flush the views cache if a _change_ in $form_state['feedapi']['enabled'] is detected (I'm winging this variable here, don't nail me on it). We should still keep feedapi and views loosly coupled so the view dependent code should be wrapped in module_exists('views').

Nice work. Really great to see this coming together.

I am OK with the dependency if it goes that way. I don't use FeedAPI without Views, but that may not be the case for everyone. If I understand correctly some of this FeedAPI architecture is heading for D7 core aggregator? Not sure if that makes a difference for upgrade path etc.

In my testing even with the admin view set to access>>permission>>administer feedapi an anonymous user can still see the view if the global permission "access all views" is selected via admin panel. Probably need to make sure that is documented.

And I don't think Views does become a dependency as long as we do not use it to replace an otherwise available admin function. In this case I don't think I can see a list of feeds to easily perform admin operations on them without Views. It doesn't break it, but makes it harder for non Views users to admin feeds.

m2c

R,
Coby

Not sure if this is because I had 1.5 installed before I applied this patch, but I am getting some errors for the following fields:

FeedAPI Feed Url - Error: handler for feedapi > url doesn't exist!

FeedAPI Feed Link - Error: handler for feedapi > link doesn't exist!

FeedAPI Feed Item URL - Error: handler for feedapi_node_item > url doesn't exist!

I think that most of these have been replaced by using the new relationship to parent feed with node xxx, but they are still showing up in my list of choices under fields.

[edit] sorry, that was for patch in #4

R,
Coby

#9 The fact that the admin section of feedapi doesn't work without views doesn't bother me so much as this one:

In my testing even with the admin view set to access>>permission>>administer feedapi an anonymous user can still see the view if the global permission "access all views" is selected via admin panel. Probably need to make sure that is documented.

This could be a real problem as it can expose unpublished feeds without the builder really being aware of it. I don't know what peope use the 'access all views' permission for, but using a view in the admin section might end up screwing up some build strategies.

Given these problems, I'd like to suggest to step back from making admin/content/feeds a view (I know, drewish, I gave you the initial go on this :-( ) or at least make it a separate issue from "full views support".

I applied the patch in #6. I could not get get the "Parent feed" to work as a filter, but it does work when given the parent feed node ID as an argument.

Edit: It does work as a filter if the "Parent Feed" relationship is not used with the filter.

@jtsnow - you should be able to establish the parent feed relationship and then filter by node id or node tile or node xyz using said relationship in the configuration settings of your filter. wow, not sure that makes sense now that i reread. hard to explain without a screenshot.

Which makes me wonder if Feed Item: Parent Feed should be removed from everything but the Relationship part of Views? Not sure.

cglusky, Yep, works great! Now that I understand relationships, it's not confusing. But, initially, having 'Parent Feed' if several places caused some confusion.

#14: I had the impression that there is a parent feed option too many somewhere - can't the parent feed filter or argument be built with a relationship now? Should we skip it therefore altogether or is it a good shortcut for a very common task? (Think of the author options on nodes, they do have an implicit relationship between node and user table as well).

alex_b,
Yes, it should all be able to be built with a relationship now. Not sure about how to best cover the possible use cases and/or if leaving the parent feed references in options besides relationship may impact functionality.
r
c

thanks drewish. comments about view permissions seem fair enough to me. i should be able to test this in the next week. have another project going with mail2web.

Nice work.

#18: I agree with your comments on #9 and #11.

In regards to #7 : FeedAPI's views integration should offer a filter "is feed type" that the default view uses. That way the admin view can be left unchanged when feed content types are added or removed. Would that work?

#23: shouldn't be necessary: #360061: Create views filter for whether a content type is used as feed or not

#19: I like renaming feedapi_standalone to feedapi. This change will cause some breakage on existing sites though, right? How could we avoid that?

#19 applied cleanly against latest dev and is working fine with a fresh D6.9 install.

Fixed/improved:

* admin/content/feed uses #360061: Create views filter for whether a content type is used as feed or not
* added description to admin/content/feed view
* change the name of feedapi_handler_filter_feed_type to feedapi_handler_filter_feed_content_type to avoid a namespace collision

#28 applies cleanly against latest dev version and works on fresh install of D6.9. lightly tested new and improved views support. nothing to report other than good job:)

Maybe this applies cleanly if you're operating a cv archive, but I got 18/1 dev out and tried to patch and noooo waaay. :(

PSEUDO-STRIKE I could really use this immediately, so if there's any danger of rolling it into dev for "easier" testing, that would be sweet. END PSEUDO-STRIKE

Please use patch -p0 < file

#27: I agree. I will put up a clear warning and upgrade instructions for the release containing this patch so that people realize that they will need to reconfigure feedapi related views when they upgrade.

Not ideal, but the naming convention proposed here is much clearer.

One thing I saw this morning was the description does not show in admin. I have the feed admin view enabled.

SS attached

#32: I'm not even sure whether this can be done: #293832: Adding a description to a menu pointing at a view. ? - wouldn't be a show stopper for this patch as the admin view is off by default anyway.

#33 agreed; not a show stopper; just a note; have no idea if it can be done either.

I did a full review of all filters, fields and arguments and they to be all sound except one:

* Fixed: had to rename *feedapi_handler_feedapi_url* to *feedapi_handler_field_url* because otherwise views couldn't find handler with the same name.

In feedapi/views/handlers we now have field handlers with the following names:

feedapi_handler_field_url
feedapi_handler_field_node_link_purge
feedapi_handler_field_node_link_refresh
feedapi_handler_filter_feed_content_type
feedapi_handler_filter_url_unique

The following will be removed from feedapi/views/handlers:
(to be removed) feedapi_handler_filter_feed_type
(to be removed) feedapi_handler_feedapi_title_link
(to be removed) feedapi_handler_feedapi_title_nid
(to be removed) feedapi_handler_feedapi_title_url

All handlers from feedapi/feedapi_node/views/handlers will be removed:
feedapi_node_handler_feedapi_item_title_url
feedapi_node_handler_feedapi_title_link
feedapi_node_handler_feedapi_title_nid
feedapi_node_handler_feedapi_title_url

New .inc files included.

#37 patch needs to be applied to dev, or on top of the previous patch(es), please? (this is rarely clear... maybe I should know, but...)

Committed.

Thanks drewish for the great work. Thanks everybody else for testing and input.