Port Views to the Drupal 7 database layer

Adding tag.

See also #286665: SEE ISSUE #1805996 -- Moving Views into Drupal Core - Same issue but in the Drupal queue. Also slightly older.

porting views to D7 and moving views into core are quite different issues..

Indeed - my bad!

I didn't read it all closely, and came here from a link about moving Views to Core.

I mistakenly created a new issue regarding this here but repost it because I think it is important to get the ball rolling again with figuring out how Views in D7 is going to work. In order to ensure that DB TNG can do everything that Views needs to do we will want to get Views completely ported to Drupal 7 as early as possible so that there is time before the code freeze to get everything working.

This is all complicated massively by the level of abstraction of Fields in Core. It is now possible to have virtual fields that can live in another database or even not in the database at all (a field could actually be pulling in data from Amazon's web services for example). This means that a full port of views and its functionality is probably not yet possible without a better API to query core for field data (which has dangerous overlaps with what we currently use Views for).

I provide a very small patch which just adds the 200 some files in views to the code registry so that views can load all of its includes. Right now I've added all of this to the views.info file but some of it may belong in views_ui.info. Even with this patch Drupal whitescreens at admin/build/views which is probably related to changes in drupal_render. As DamZ pointed out on IRC "drupal_render can't be called from a theme function on the same object anymore, this results in infinite loops".

Perhaps this should have its own team? Lets get the ball rolling!

Related discussion to some of these issues has been going on here: http://groups.drupal.org/node/19097 and at length in IRC.

/me wonders where my subscribe button is.

@mfer here: #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment

@tizzo: Thank you for getting the ball rolling! I'm looking forward to testing for new bugs once this gets off the ground.

@merlinofchaos: Is there a HEAD version that this issue could be added to?

That is surely the biggest .info file ever! I'm only subscribing for now, but I'll see what I can contribute in a bit.

Incidentally, would an extra repository make sense for this work in progress (as we had for the initial DBTNG change I believe)? The port patch will surely be hundreds of KB, and if all development has to be via file upload in this issue, it will soon become painful. Especially if people want to work on small components like hook_perm().

Picked the patch from #5 up.
Done:
changes to hook_perms.
And I changed the one reference to function_exists() to drupal_function_exists(); that was only in the docs, though.

@merlinofchaos: Is there any way you could create a D7 branch and commit this interim patch from time to time? Otherwise I fear it will soon become un-reviewable, especially when someone starts with the DBTNG stuff.

Anyway, here's the patch.

There is a lot of path-finding and including going on, especially on the plugin and theme handling. Views practically has its own little code registry, likely made obsolete by the code registry in core.

I can try to snip away some of this, but it'd be tricky to test it while the rest of Views isn't yet functional.

---

Speaking of the code registry, Views also heavily relies on implementation at a distance (eg. implementing "node_views_plugins"). That won't work until this issue (happily RTBC) gets in: #451152: Implementing a hook on the behalf of another module . At a glance, I don't think it will require workarounds on the Views side - as far as I can tell, the patched registry will scan code files for all prefixes to achieve the same behavior as in D6, instead of scanning only for the module's own prefix.

I've tried to clear out a lot of the include handling and replace it with code registry invocations - but I'm far from done and the patch is already beyond 40k. Furthermore, I'm doing this blindly, and possibly naively. Nevertheless, here is a patch that builds on #11.

You are right in that this development definitely shouldn't be moved to a kind of "backroom process". The patch-review-commit process we've got here is very healthy for spotting regressions before they happen, as well as making everyone familiar with the code. My suggestion for any off-site repository would therefore be to post the interim patches here for review, and keep the maintainer model so only one person (merlin, presumably) can sign off on then.

On the other hand, if we do that then it could make sense again to use the d.o repository after all. Unlike Fields, this is a contrib project whose HEAD we can break at will without holding up all core development. We would just have to add a textfile along the lines of "DO_NOT_USE-D7_ONLY.txt" for people who deploy unstable CVS code on their D6 sites.

Updated the patch in #13.
Done:
Some #process functions have been renamed
Rebuild functions have changed names
The hook_menu() and hook_theme() "file" and "file path" keys have been removed.

I also posted a wiki page on groups.drupal.org to keep track of this issue: http://groups.drupal.org/node/22346

Missed some files last time. Try this one.

Will the new role_permission table affect us at all? I can't see any database query accessing it, and the user_access API has remained the same.

However, the #value -> #markup change requires some fixing. I've changed (hopefully) all form elements that used #value to show HTML markup. This patch also adds a README_DRUPAL-7.txt file that warns users to checkout the (as yet non-existent) DRUPAL-6--2 branch for the D6 version. This makes things much easier for bleeding-edge CVS deployment.

Now approaching 80k, but then, Views is a very big module with roughly 30,000 lines of code.

This patch builds on #18 and also replaces all calls to time() with REQUEST_TIME.

Note that microtime() is left unchanged, so if this patch must be rerolled be sure to use s/([^o])time()/$1REQUEST_TIME/g to limit it to time().

Done:
Replace drupal_clone() with clone

if this patch must be rerolled be sure to use s/([^o])time()/$1REQUEST_TIME/g to limit it to time()

I didn't get that, so I didn't do anything of the sort. Just to let you know.

I was referring only to a potential redo of that part of the patch. Since by now we're piling changes upon changes, we're pretty much screwed if one of the early ones must be rolled back. Then it's a lot of juggling around with diffs to take the change out and leave the rest in - or we need to redo some of the easier text replacements from scratch.

For example, I notice that I naively deleted views_include, believing it to be obsolete due to the code registry. While the code registry indeed *can* take it over, it is costly to look a function up in the registry when you already know where it is because it's part of your own module. So while some of the plugin include handling should be replaced with code registry, I am fairly sure that Views' own include files should be loaded by name.

That means I have to revert some of the changes of #13 out of #20. I'll get on that tomorrow...

Is someone in this thread willing to be responsible for commits? I simply don't have the time to follow a Drupal 7 branch, but I am willing to create one and give maintainer access, as long as it's someone trustworthy who will promise to commit only in that branch.

I can do trustworthy, but I don't know if I can do active, for however long this will take... ;)

Here is a patch based on #20 that undoes some of the over-eager deletions from #13, and instead keeps explicitly including Views' own include files.

Things made obsolete by the code registry are limited to:

- theme and menu callback filenames, because theme and menu use the registry
- including handler class files, because class_exists auto-loads.

In views_include_handler, I removed the require and path code while still making sure parent classes are included (because it seems that "parent" is not the same as "super-class").

Okay, this is really confusing.

Apparently 'parent' /is/ the super-class, and the auto-loading should make it redundant. Also, views_module_include() includes Views' own code files, but those files only contain "exported" hooks implemented on behalf of other modules. Hooks are loaded by the registry, so views_module_include() and all that references it should go.

However, I haven't fully understood the scope of all that yet, and need to sleep on it first.

Okay, this update does for views_module_include and all its callers. It seems that it was only used before invoking hooks, which the registry would now take care of.

I've also changed the code in views_include() to call drupal_module_include('inc', 'views', "includes/$file");, which automatically finds the right path.

Code inclusion for class files would be next. Also, views_microtime() should die...

That was more straightforward than I thought; I didn't need to bank my changes in #25.

- views_include_handler() is axed, and its only caller now is satisfied with class_exists() which should load it and all super-classes.
- views_microtime() is dead and all its callers now call the PHP5 microtime(TRUE) function to get the float time.

I think a lot of the declaration of "parents" for handler classes (which seem to be only super-classes of the class in question) can be removed now. Please correct me if they're needed elsewhere, merlinofchaos.

Trivial change of the filter label for comment.status from "Moderated" to "Published" and the default from 0 to 1, following the flip of the boolean values.

Note: This is one of the changes that must be included in the upgrade path for custom views.

This one is a rather substantial (and questionable) change, wherein the API of hook_views_handlers() is changed to expect only an array of classes. So far, the API accepted paths, filenames and parents as meta-data, but the auto-loading made all these obsolete so the class names should be enough.

We've only got one significant task on the UNSTABLE-1 list left, and it's quite a whopper.

I'm talking, of course, about DBTNG. Switching Views from writing SQL to using PDO will take quite a lot work, and it's honestly beyond me. I'll gladly take part in joint development, but this is not something I can simply write, patch and upload here on my own. Maybe there are some other small porting tasks to be taken care of, but before long we're going to have to confront the database stuff. :)

Well as we've ticked off everything else, and you magically made the registry thing work (big kudos btw), maybe we could get some of those crazy DB:TNG people involved in this issue. In the end, this issue is something pretty much every Drupalista will profit from, so ...
I, too, am more a guy for smaller porting tasks, so I'm asking, whether anything speaks against starting with things from UNSTABLE-2, or would you want to get this working more or less stable (how ironic...) on UNSTABLE-1 with the registry and DBTNG?

On that note

I, too, am more a guy for smaller porting tasks, so I'm asking, whether anything speaks against starting with things from UNSTABLE-2, or would you want to get this working more or less stable (how ironic...)

Actually we're better off developing for HEAD than the various UNSTABLEs anyway, so any of the other fixes are fair game to do before DBTNG. It may even be a good move to just hack the SQL into a working shape before replacing it with pure query-building (webchick's words of wisdom). One of the objectives is to get basic functionality back as soon as possible, because for ease of development, nothing beats being able to test your changes. ;)

I agree. Drupal 7 doesn't *Require* dbtng to work, therefore we should actually consider it a non-critical change at this point.

Arancaytar: You are correct, handler parentage is solely for include files. So yay, not needing that anymore.

Good news! I've hammered my working copy of Views into a shape that can be enabled without even a warning-level error!

Bad news: I was a bit overzealous when ripping out code inclusion - hook_theme still needs the path key for template files, as the code registry only discovers functions. I've put these patchs back in in this patch.

Other changes in this patch: Views is using a forked Form API. Because it put this right into the drupal_ namespace, it is unsurprising that it caused a collision - drupal_build_form is now in core. I've moved all the drupal_ functions into the private _views namespace.

TODO: Unfortunately, these namespace functions just make the module install without errors. admin/build/views cannot yet be properly rendered. This is because the forked form.inc ties partly into the D6 form.inc, and since these ties are not part of the public API they are tricky to update for D7. Since the forked functions are changed only minimally, the easiest solution may be to examine D7 form.inc and base a new fork on that (if it is necessary; forks are ugly).

I have started to do some detective work on the partial Views Form API fork, hoping to find what it changes, how much of this is still needed in D7, etc. It is essential that we take care of this while core is still "hot", because right now there is a good chance of sensible Form API enhancements being canonized into core. That would obliviate the need of a fork, making everyone much happier.

To make diffing easier through all the refactoring, I wrote a function-splitter that copies each Doxygen-documented functions from one big code file into a function file in another directory. The splitter and the results of some diffing are here, in case anyone else wants to help investigating Views' form handling. Please share what you find, and whether you think we can work around it in D7. :)

One thing I've already found is that Views needs to force repeated form validation on certain forms:

+ * The original version of drupal_validate_form does not have an override for
+ * the static check to only validate a form id once. Unfortunately, we need
+ * to be able to overridet his.
  */
-function drupal_validate_form($form_id, $form, &$form_state) {
+function drupal_validate_form_new($form_id, $form, &$form_state) {
   static $validated_forms = array();
 
-  if (isset($validated_forms[$form_id])) {
+  if (isset($validated_forms[$form_id]) && empty($form_state['must_validate'])) {
     return;
   }

This functionality is used in only a single place, the exposed widget form.

// ------------------------------------------------------------------
// Exposed widgets form

/**
 * Form builder for the exposed widgets form.
 *
 * Be sure that $view and $display are references.
 */
function views_exposed_form(&$form_state) {
  // Don't show the form when batch operations are in progress.
  $batch =& batch_get();
  if (!empty($batch)) {
    return array(
      // Set the theme callback to be nothing to avoid errors in template_preprocess_views_exposed_form().
      '#theme' => '',
    );
  }

  // Make sure that we validate because this form might be submitted
  // multiple times per page.
  $form_state['must_validate'] = TRUE;

Number two, the forked drupal_rebuild_form says that all it does is avoid caching forms that don't want to be cached. This confuses me at first glance, because it sounds like a bug in core. However, drupal_rebuild_form is a rarely-called function, used only in multistep or AHAH, and perhaps forms can simply be cached here.

Here is the change:

+ * This change merely respects a form's wishes not to be cached.
  */
-function drupal_rebuild_form($form_id, &$form_state, $args, $form_build_id = NULL) {
+function drupal_rebuild_form_new($form_id, &$form_state, $args, $form_build_id = NULL) {
   // Remove the first argument. This is $form_id.when called from
   // drupal_get_form and the original $form_state when called from some AHAH
   // callback. Neither is needed. After that, put in the current state.
@@ -51,16 +19,18 @@ function drupal_rebuild_form($form_id, &
   $form['#build_id'] = $form_build_id;
   drupal_prepare_form($form_id, $form, $form_state);
 
-  // Now, we cache the form structure so it can be retrieved later for
-  // validation. If $form_state['storage'] is populated, we'll also cache
-  // it so that it can be used to resume complex multi-step processes.
-  form_set_cache($form_build_id, $form, $form_state);
+  if (empty($form['#no_cache'])) {
+    // Now, we cache the form structure so it can be retrieved later for
+    // validation. If $form_state['storage'] is populated, we'll also cache
+    // it so that it can be used to resume complex multi-step processes.
+    form_set_cache($form_build_id, $form, $form_state);
+  }
+
+  // Originally this called drupal_process_form, but all that happens there
+  // is form_builder and then submission; and the rebuilt form is not
+  // allowed to submit. Therefore, just do this:
+  $form['#post'] = $form_state['input'];
+  $form = form_builder($form_id, $form, $form_state);
 
-  // Clear out all post data, as we don't want the previous step's
-  // data to pollute this one and trigger validate/submit handling,
-  // then process the form for rendering.
-  $_POST = array();
-  $form['#post'] = array();
-  drupal_process_form($form_id, $form, $form_state);
   return $form;
 }

This added functionality is used on the AHAH forms when configuring a view. Not used anywhere else though. (AHAH is something I still haven't grokked, so I'll need a lot of help with this.)

Sorry for spamming your inboxes, webchick, merlin et al. It's just easier to deal with this one comment at a time. :)

Here is the third forked method, drupal_process_form. It says that it allows overriding the redirection, and forces GET forms to be sent for submission processing.

Beyond that, it calls the forked validator (see #36) that forces the exposed widget form to be revalidated.

+ * Views' replacement for drupal_process_form that accepts commands
+ * not to redirect, as well as forcing processing of 'get' method forms.
  */
-function drupal_process_form($form_id, &$form, &$form_state) {
+function drupal_process_form_new($form_id, &$form, &$form_state) {
+  // submitting, and handling the results returned by its submission
+  // handlers. Submit handlers accumulate data in the form_state by
+  // altering the $form_state variable, which is passed into them by
+  // reference.
   $form_state['values'] = array();
 
+  // With $_GET, these forms are always submitted.
+  if ($form_state['method'] == 'get') {
+    if (!isset($form['#post']['form_build_id'])) {
+      $form['#post']['form_build_id'] = $form['#build_id'];
+    }
+    if (!isset($form['#post']['form_id'])) {
+      $form['#post']['form_id'] = $form_id;
+    }
+    if (!isset($form['#post']['form_token']) && isset($form['#token'])) {
+      $form['#post']['form_token'] = drupal_get_token($form['#token']);
+    }
+  }
+
   $form = form_builder($form_id, $form, $form_state);
   // Only process the form if it is programmed or the form_id coming
   // from the POST data is set and matches the current form_id.
   if ((!empty($form['#programmed'])) || (!empty($form['#post']) && (isset($form['#post']['form_id']) && ($form['#post']['form_id'] == $form_id)))) {
-    drupal_validate_form($form_id, $form, $form_state);
+    drupal_validate_form_new($form_id, $form, $form_state);
 
     // form_clean_id() maintains a cache of element IDs it has seen,
     // so it can prevent duplicates. We want to be sure we reset that
@@ -66,8 +72,13 @@ function drupal_process_form($form_id, &
       // however, we'll skip this and let the calling function examine
       // the resulting $form_state bundle itself.
       if (!$form['#programmed'] && empty($form_state['rebuild']) && empty($form_state['storage'])) {
-        drupal_redirect_form($form, $form_state['redirect']);
+        if (!empty($form_state['no_redirect'])) {
+          $form_state['executed'] = TRUE;
+        }
+        else {
+          drupal_redirect_form($form, $form_state['redirect']);
+        }
       }
     }
   }
 }

I haven't checked the calling code in Views, but it's pretty certain that these changes are for the sole benefit of the exposed widgets, which work with GET.

These are the three forked form functions. Most of drupal_get_form() has been forked into _drupal_build_form() in Views, with a few differences.

In summary, we need to find a way to do these (or avoid the need for them) in D7:

- force get-submitted forms to be sent for submission processing (without having to inject form tokens into the U query string, since the very idea of the GET is to produce persistent URLs), and be validated properly.
- force AHAH forms not to be cached on rebuilding, or work out how to clear the cache, or work out how to deal with a cached form.

These trivial problems are left as exercises to the reader. :)

Yay!

I suspected this when I examined D7 drupal_build_form, but didn't dare hope - all of Views' fork is indeed part of D7 FAPI! That means that rather than reforking form.inc, we are left with the relatively simpler task of ripping out the fork and using the normal core API. Yay indeed.

Yet the time I spent doing this investigation wasn't wasted, because I know a lot more about D7's form API now than I used to, and I also got some practice in hunting down code changes. :)

Edit: Thank you, webchick! And yet again crossposted. (I'm removing Form API now, because simply ripping out the customized code doesn't involve such intimate knowledge of the Form API as I thought).

D7 will require dbtng syntax but I don't think there'll be a hard requirement to use db_select() for query building - at least at the level of things exploding. Having said that, that really ought to happen before code freeze if possible, so that if there's limitations in dbtng which make that impossible (relationships are likely to be hard), we know about it in time to fix it.

Subscribing, by the way.

Started with UNSTABLE-2.
@ Arancaytar: If you've started with the ripping out of the form API fork, that's not too much of a deal, only required changes to two lines of code, up to now.

Done:
# Use absolute path (constructed from DRUPAL_ROOT) when including a file

Started with UNSTABLE-3, then. Still only a few lines of code touched.

Done:
Changed parameters for drupal_add_js() and drupal_add_css()

DBTNG's extension system was designed specifically with Views in mind. I'd love to make sure that it actually works for that use case. :-) Grab me in IRC if you need a tutorial.

And subscribing...

tstoeckler et al: Feel free to upload bigger changes; I haven't done anything locally yet. (This illustrates our need for some kind of repository, though - patch on patch loses all the benefits of concurrent development. ;) )

Crell: I'll gladly take you up on that. I need to be somewhat more awake than I am right now, though. :)

I'd love to see a 7.x branch on CVS and it'd be a real shame for things to go in an external repo, Arancaytar are you up for maintaning it? Presumably as code freeze looms closer there's a chance merlinofchaos will have more time to look at it anyway?

My main concern about a 7.x branch is that now there is a 3.x and a 2.x branch and changes will need to be synced up with the patch. I know keeping the patch rolling is kind of a pain, but it seems like it would be easier to manage the patch with git or bzr than actually creating a branch, because it has superior merge capabilities and allows people ot have their own private repos and toss stuff back and forth.

Maybe we can get sdboyer to show us how to set something up to do this?

Either that or both the external repo and regular syncs with CVS. That way people can checkout Views 7.x and try it out (and post small patches against CVS).

That's a good idea, catch. I'll happily work with that but I need someone else to actually do it. I currently don't have the free bandwidth to do the administration.

How about creating a temporary, separate project for all the benefits of d.o's issue queues, cvs repo, and the local infrastructure team?

CVS doesn't sync well with CVS. =(

I'm not sure I have grasped the points here yet.

My main concern about a 7.x branch is that now there is a 3.x and a 2.x branch and changes will need to be synced up with the patch. I know keeping the patch rolling is kind of a pain, but it seems like it would be easier to manage the patch with git or bzr than actually creating a branch, because it has superior merge capabilities and allows people ot have their own private repos and toss stuff back and forth.

Either that or both the external repo and regular syncs with CVS. That way people can checkout Views 7.x and try it out (and post small patches against CVS).

So, should we commit these changes here to HEAD before it's finished, or should we not? Do you anticipate many changes to the D6 branches that will require up-porting, and when would be a good cut-off date for branching? :)

---

I'll try get the forms stuff out tonight, based on the patch in #44.

Here we go. Since the new Form API tells us to use drupal_build_form the same way we would use drupal_get_form, except for the option of passing in a form state, I just replaced _views_build_form (already changed) with drupal_build_form.

The only non-obsolete function in includes/form.inc is the dependency processing callback; this is moved into views.module in order to get rid of the form.inc file.

I haven't noticed any place where a form was concatenated to other output - any such occurrences would need to be adjusted for the change that forms seem to be returned in array form now.

chx pointed me here to help out, as I'm working on a backport of the D7 DBTNG for Drupal 6:

chx: mikey_p: what you want here is talk to Arancaytar and tstoeckler and merlinofchaos backport the D7 port of Views based on this for Views 3.x
chx: http://drupal.org/node/363410

Currently the backport module is living in my sandbox at http://cvs.drupal.org/viewvc.py/drupal/contributions/sandbox/mikeyp/dbtng/

It seems to be mostly working (I haven't tested it thoroughly yet), but I still need to add back some of the colliding functions (such as db_set_active and so on).

I would be willing to do some review + commit work (here in CVS; probably HEAD). This would require

- a go from merlinofchaos

- periodically merge-patches to account for changes in 2.x/3.x

However, either way, this patch could probably be much smaller if it would not contain coding style clean-ups like this:

-    'style' => array('view' => NULL, 'options' => NULL, 'rows' => NULL, 'title' => NULL),
-    'row' => array('view' => NULL, 'options' => NULL, 'row' => NULL, 'field_alias' => NULL),
+    'style' => array(
+      'view' => NULL,
+      'options' => NULL,
+      'rows' => NULL,
+      'title' => NULL,
+    ),
+    'row' => array(
+      'view' => NULL,
+      'options' => NULL,
+      'row' => NULL,
+      'field_alias' => NULL,
+    ),

Please leave the coding style alone. Clean-ups can follow later. Only change what is really necessary to change.

Besides, we generally put the array that represents theme arguments on one line because they resemble function arguments and the visual cue is important. So please do not change that style, that's actually incorrect.

I'll definitely help out with reviews (at least), as long as it's on d.o and normal issue queue. A combination of sun and Arancaytar on commits seems like a safe bet to keep things moving.

@webchick : actually I don't think #392494: Fields API needs to allow searching of scalars is related to Views. It is meant to allow simple queries like "get all objects where field_foo = 'bar' " *without* Views - primarily for housekeeping tasks (cache clean...) or simple features (autocomplete...).

Field API Views integration will most likely be be quite similar to what is currently done in CCK D6, except it is provided by each field storage engine. The 'default' sql storage will expose its tables to Views, which can then build its direct queries, the way it always did. If Views stays contrib, it's probably going to have to expose data on behalf of the core sql_field_storage.module).

Alternate field storage modules (SQL based or not) will expose their own Views data for the fields they handle :
- For (local) SQL-based engines, they just expose their own tables.
- The case of remote storage engines will probably be handled similarly to #293841: Allow different back-ends for Views. Field-related aspects are discussed in #443422: 'per field' storage engine - which I strongly encourage anyone interested in D7 Views / Fields to read and chime on.

Rerolled to apply to latest Views-HEAD.

Phew, that was some piece of work. The Commit a D7-branch vs. go to an external repo vs. whatev debate should really be settled quickly for this not to happen every other day.

Sorry, but independent from merlinofchaos' decision, my last comment stands - all of the following is mostly cruft we have to avoid:

-    'arguments' => array('tags' => array(), 'limit' => 10, 'element' => 0, 'parameters' => array()),
+    'arguments' => array(
+      'tags' => array(),
+      'limit' => 10,
+      'element' => 0,
+      'parameters' => array(),
+    ),
...
-    'style' => array('view' => NULL, 'options' => NULL, 'rows' => NULL, 'title' => NULL),
-    'row' => array('view' => NULL, 'options' => NULL, 'row' => NULL, 'field_alias' => NULL),
+    'style' => array(
+      'view' => NULL,
+      'options' => NULL,
+      'rows' => NULL,
+      'title' => NULL,
+    ),
+    'row' => array(
+      'view' => NULL,
+      'options' => NULL,
+      'row' => NULL,
+      'field_alias' => NULL,
+    ),
...
-      $function = $module . '_views_api';
-      $info = $function();
+      $info = module_invoke($module, 'views_api');
       if (isset($info['api']) && $info['api'] == 2.000) {
-        if (!isset($info['path'])) {
-          $info['path'] = drupal_get_path('module', $module);
-        }
...
-dependencies[] = views
+dependencies[] = views
\ No newline at end of file
...
 
   $convert = array('file' => 'includes/convert.inc') + $base;
-
   $items['admin/build/views'] = $base + array(
     'title' => 'Views',
...
@@ -45,7 +45,7 @@ class views_handler_argument_date extend
           continue;
         }
       }
-  
+
       if (arg(0) == 'node' && is_numeric(arg(1))) {
         $node = node_load(arg(1));
       }
...
-        '#prefix' => '<div class="description">',
-        '#value' => t('This item is currently exposed. If you <strong>hide</strong> it, users will not be able to change the filter as they view it.'),
-        '#suffix' => '</div>',
+        '#markup' => '<div class="description">' . t('This item is currently exposed. If you <strong>hide</strong> it, users will not be able to change the filter as they view it.') . '</div>',
...
@@ -47,6 +48,7 @@ function views_views_plugins() {
         'admin' => t('Page'),
         'help topic' => 'display-page',
       ),
+
       'block' => array(
         'title' => t('Block'),
         'help' => t('Display the view as a block.'),
@@ -60,6 +62,7 @@ function views_views_plugins() {
         'admin' => t('Block'),
         'help topic' => 'display-block',
       ),
+
       'attachment' => array(
         'title' => t('Attachment'),
         'help' => t('Attachments added to other displays to achieve multiple views in the same view.'),
...

And some more changes in hook_theme() could be avoided by keeping:

-  $path = drupal_get_path('module', 'views');
-  require_once "./$path/theme/theme.inc";
-
-  // Some quasi clever array merging here.
-  $base = array(
-    'file' => 'theme.inc',
-    'path' => "$path/theme",
-  );
+  // Theme functions are discovered, but template paths must be specified. 
+  $path = drupal_get_path('module', 'views') . '/theme';

Additionally, I'm not sure whether the changes in help files were intentionally.

A port to D7 should do the minimum to get Views working. Any coding-style and/or code clean-up changes have to be done separately, to be concurrently applied to 6.x and 7.x.

Corrected everything in #65, except for:

-        '#prefix' => '<div class="description">',
-        '#value' => t('This item is currently exposed. If you <strong>hide</strong> it, users will not be able to change the filter as they view it.'),
-        '#suffix' => '</div>',
+        '#markup' => '<div class="description">' . t('This item is currently exposed. If you <strong>hide</strong> it, users will not be able to change the filter as they view it.') . '</div>',

This came from:

Use '#markup' not '#value' for markup.

from the module upgrade instructions in #18.
Even though it might be minor I think we should stick to the upgrade guide to keep an overview.

Hope it still applies...

One line of code changed:
# Removed FILE_STATUS_TEMPORARY

A lot of stuff from UNSTABLE-4 doesn't affect Views. The rest is over my head.
Updated http://groups.drupal.org/node/22346.

Done:
# JavaScript should be compatible with other libraries than jQuery

...Slowly running out of these trivial things, soon I'll have to get my hands dirty :(

Hello world, i opened http://workspace.activestate.com/projects/drupal7views and applied the patch, i got two rejects. please register at workspace and drop me a mail (chx1975 gmail com) and i will add you to the project. I am working on DBTNG.

subscribe

error when access to http://localhost/d7/admin/build/views

* Notice: Undefined index: args in drupal_retrieve_form() (line 417 of D:\xampp\htdocs\d7\includes\form.inc).
* Warning: array_merge(): Argument #2 is not an array in drupal_retrieve_form() (line 446 of D:\xampp\htdocs\d7\includes\form.inc).
* Warning: Missing argument 1 for views_ui_add_form() in views_ui_add_form() (line 544 of D:\xampp\htdocs\d7\sites\all\modules\views\includes\admin.inc).
* Notice: Undefined variable: form_state in views_ui_add_form() (line 545 of D:\xampp\htdocs\d7\sites\all\modules\views\includes\admin.inc).
* Notice: Undefined index: args in drupal_retrieve_form() (line 452 of D:\xampp\htdocs\d7\includes\form.inc).

Thanks for your port.
-----------------------

I started to work on the query builder but ran into #451152: Implementing a hook on the behalf of another module . I'm postponing any further work on that part until #497118: Remove the registry (for functions) is resolved and we've settled on an autoloading strategy for classes. chx has done some good work on the query builder already, which I expect to see him post to this issue very soon per webchick's plea in #69. :)

A lot of the changes we're done with were based on ripping out code inclusion to replace it with the function registry... :/

I'm going to checkout HEAD and svn and try to catch up with what's been happening.

Edit: I see svn is conflict-free with HEAD right now - however, drupal_function_exists is called in several places, and that has to change. At least all of Views' handler and plugin structure is class-based, and can still take advantage of the auto-loader.

Some things I noticed on simply going ahead and installing:

  if ($GLOBALS['db_type'] == 'pgsql') {

Apparently this global isn't used anymore. It's still referenced on the Docs page, but not actually set anywhere in the code.

Also, the class registry understandably chokes on the duplicate class in modules/node/views_handler_argument_node_language.inc and modules/translation/views_handler_argument_node_language.inc. They're identical, so I'm just testing further after deleting one of them.

subscribe

reading the issue queue would have helped.

I had an idea: use the backport of the dbtng
So we could concentrate on the views query builder stuff, without suffering from all other changes form drupal7.

@derine: merlin and I have been talking about that at DrupalCon. I'm going to start working on that for Views 3 in Drupal 6, which will introduce a dependency on dbtng.module. The D7 port should then be much easier.

@crell

Is there anything which could be done for this specific part?

I'll have a better sense of that in a few days after I get the core port done. chx is also going to be working on unit tests. (He's sitting next to me as we speak, er, type.)

Then this issue must be tagged as Views 3 no ?

So as a more complete update, here's the current roadmap:

1) Port Views 3 to the dbtng backport module. Adjust APIs as necessary to do so.

2) That also introduces a dependency on the autoload.module. That means we can make the appropriate changes to leverage PHP autoloading using the autoload module. Adjust APIs and code as necessary to do so.

3) That introduces a dependency on PHP 5. Adjust APIs and code as appropriate to take advantage of that fact.

4) Make other intended API changes to Views 3. See merlin's earlier post for a list of what we want to do.

5) Release Views 3 for Drupal 6. There will be much rejoicing.

6) Port Views 3 to Drupal 7. With all of the hard work done above, this should be a fairly simple process with, hopefully, few if any changes to the Views API.

7) PROFIT!!!!!

What "adjust/change APIs as necessary" means will be figured out as we go. :-)

Note that the CTools changes mentioned in my roadmap may be paused so that it doesn't introduce a new dependency for D7. While CTools to d7 would be a great thing, it is not necessary and I think people would rather have Views.

Oh yes, and if you want to see the ongoing work, the bzr repository is here:

https://code.launchpad.net/~views-port/+junk/Views3-d6

Here is a branch which does the autoload conversion: https://code.launchpad.net/~dereine/+junk/v63-autoload

With clicking around everything works fine.

I'm really just playing around, but I've managed to get a basic DBTNG-built query to work with some changes to the default query plugin and the view_join handler in handlers.inc.

My branch is at http://code.launchpad.net/~arancaytar/+junk/views-dbtng-experimental. I based it on dereine's autoload branch. None of the obsolete code parts are removed yet (the ones that clashed with variables are commented out), in order to make it easy to see the additions.

With this code, I was able to create and display a very primitive {node} view that has no filters, but joins with the {node_revision} table and orders by nid.

I anticipate that conditions will be one of the substantial chores here, as all filter plugins pass printf-style SQL snippets and arguments to add_where(), and we'll probably have to convert that to either add_where($field, $value, $operator) or add_where(QueryCondition $clause) or both.

The conditionals weren't actually as much work as I'd feared - in the standard handlers, it was fairly straightforward.

Here's my second attempt, branched at http://code.launchpad.net/~arancaytar/+junk/v2-conditionals. With this code, I created a custom node view with basic field display (including body from node_revision) which is ordered by nid and also filtered by the node title containing a specific word.

However, I came across the problem that query.inc is not loaded (and db_condition not defined) until dbtng_select() is called, which means QueryConditions DatabaseConditions cannot be defined until the SelectQuery at least has been instantiated.

That in turn means the idea of allowing filter handlers to pass QueryCondition DatabaseCondition objects into a view (the best way to allow complex clauses) clashes with the concept of only calling dbtng_select() when the query is built for execution. Maybe the SelectQuery object should be constructed as soon as possible. Simple filters work now, but handlers cannot yet call db_condition() on their own.

Edit: This is still playing around, mind. I hazard a guess that Crell is further along on this already...

Is there a way we can do this using 'prepare' (see http://www.php.net/manual/en/pdo.prepare.php) to create the conditions before we're ready to actually run the query? If that is not already in the Drupal dbtng API, which I think it is not, maybe we need an issue to add it.

I don't know enough about dbtng to do more than suggest the possibility.

Well, running dbtng_select only constructs the query builder (and db_condition constructs a DatabaseCondition), apparently without involving PDO just yet.

Good grief you guys are quick. I'm still on vacation in Paris. :-)

I've not looked at the code yet, but the main question to work out (as of Saturday when merlin and I were last discussing it) is whether the Views query object should extend views_plugin_query or SelectQueryExtender. We want to implement it as an extender, because DBTNG was designed to work that way, but there's behavior we want to include from both parent classes. That means we need to extend one and re-implement the code from the other, and declare both interfaces. (views_plugin_query doesn't have a formal interface yet, but it should eventually.) I figured that it would be easier to re-implement views_plugin_query, since it's much smaller and it means fewer method calls on the stack.

The idea is that the Views DB query should have the exact same API as the core DB query builder, and then whatever extra behavior it needs. (ensure_table(), for instance.) That means we switch the actual query building methods to the method signatures that the SelectQuery class uses. With extenders that should be easy. That way, pretty much all cross-DB behavior is handled internally by DBTNG and Views doesn't have to worry about it. It also means DB-based handlers can use the exact same API as all other built queries in Drupal, which reduces mental overhead and allows autoloading to work.

It is true that DBTNG does not typically construct a PDOStatement object until the last second. That's because we do not know what the query string will be until the built query is compiled.

My gut feeling is that we should stick with views_plugin as the base class and pass through to our own extension of SelectQueryExtender -- for one, this allows us to not have to change the Views API at all except where there is no choice, and instead make changes in the plugin to massage data from the way Views currently wants it to how DBTNG wants it. Then, when everything is working, we can go through and reconcile differences.

The idea is that the Views DB query should have the exact same API as the core DB query builder, and then whatever extra behavior it needs. (ensure_table(), for instance.) That means we switch the actual query building methods to the method signatures that the SelectQuery class uses. With extenders that should be easy.

Ah, that would be lovely! :D

Yeah, views_plugin seems easier to reimplement. I suppose the methods of its parent views_object would have to be wrapped as well (there aren't many, though).

merlin: Does the views_query_plugin database-version align with other implementations? Vis, is add_where() constant between them, or would that vary per-plugin? Because if it varies per plugin I'd much rather unify the API than force people to remember two different query building APIs.

It is specific to SQL.

My reasoning, though, is that you can get something working a lot faster if you don't change query::add_where().

Otherwise, you've got to modify every handler...

True, but we can modify add_where() to use $this->condition() or $this->where() internally. That's probably easier, in fact, than modifying condition() or where(). add_where() lives in the extender and is just a wrapper for the DBTNG API calls. Both then work. We can then encourage people to move to the native calls as they will be faster.

We can then remove the add_where() (et al) wrappers later, either before Views 3 goes stable or as part of the D7 port, or whenever.

The handlers calling add_where do have to be modified eventually, if only because passing printf-style placeholders and argument arrays isn't done anymore. If they're changed to add_where($field, $value, $operator) syntax, add_where() becomes just an alias for condition(), and updating the function name is just a find-and-replace job.

sub

Sub.

The current development is on https://code.launchpad.net/~d7cx/d7cx-views/DRUPAL-7--3
Come to #d7cx and you will find something todo :)

The current code lives in the launchpad repository. The basic query object is working, even if I'm pretty sure that the joins are still broken (we have no tests on relationships yet).

The signature of add_where and add_having had to be changed, because there where a SQL snippet with placeholders. It is temporarily:

function add_where($group, $field, $value = NULL, $operator = NULL);
function add_having($group, $field, $value = NULL, $operator = NULL);

<?php
bzr diff -r 1
?>

Updated patch, some work from chx.

There is now an initial commit of the work for the Drupal 7 port of Views 3 in CVS in the DRUPAL-7--3 branch.

Future discussion, patches, etc, should be posted against the 7.x-3.x version in their own issues.

Surely we can add the #D7CX tag now ;)
This is awesome to see.

added a current tgz

Is there a definitive todo list for what needs to be done to get a stable D7 release or is it a matter of looking at all issues posted against 7.x?