Allow sort by user names

OK, here it is.

First, a demonstration:

http://blue.live4all.co.il/~mooffie/cms/node/15

Click the "Who's bookmarked this page" tab. The columns are sortable; that's the main purpose of this patch.

(Note that I've spiced up that view with a 'Search user:' exposed filter. You can ignore that. That's a tiny addition I haven't included in the patch, just to show that Views is a good choice for building this tab.)

And the patch.

I said it'd be 'trivial', but it turns out I had to re-engineer the current 'Bookmark: User for xyz' field. So this patch is bigger than the few lines I initally thought it would take. I'll shortly explain it.

Now, for the explanation:

I had to re-engineer the way the current 'Bookmark: User for xyz' field was implemented.

Here's why:

To be able to sort by username we have to pull in the 'name' column from the 'users' table. There are two methods to 'pull in' fields in Views:

1. The straightforward method is to describe the table containing the field(s) to Views. Then the admin simply includes whatever fields he likes in his view; or:
2. To pull the data using $query->add_table() and $query->add_field().

The second method is what's currently used in ViewsBookmark to show the 'Bookmark: User' field, and, unfortunately, it has some limitations, especially concerning sorting: we can't click-sort fields that we haven't described to Views. You may say "Ok, let's ditch click-sort. We can still use a hard-coded sort, in the 'Sort Criteria' box," but then things aren't yet pretty: because of the way ViewsBookmark tables are exposed to Views we'll have to clutter the UI with a separate 'Sort Criteria' for each bookmark.

But perhaps the main point is that method #1, "The straightforward method," is simply the most elegant way to solve all problems --present and future. And it's clean; I even had to _remove_ more code than I added! The current code was: (1) ad-hoc for only showing the user; (2) a bit sloppy.

The bad news is that we now have to provide an upgrade path (views_bookmark_update_5150). The good news is, that it's a safe route and we aren't likely to receive complaints from users about their views breaking, because even if 'update.php' isn't visited nothing bad happens: if a view uses the old 'Bookmark: User' field, it will now show the numeric user IDs instead of their names. Once 'update.php' is run this too is fixed.

Now, you may be asking youself:

"Is this worth the bother? All this just for being able to sort by usernames?!"

IMHO, the answer is a resounding 'yes!'.

First, I consider ViewsBookmark a workhorse. It has diverse uses, and being able to drill-down and sift users is a basic task.

Second, this patch very much falls under the "code cleanup" category.

We can still use a hard-coded sort, in the 'Sort Criteria' box," but then things aren't yet pretty: because of the way ViewsBookmark tables are exposed to Views we'll have to clutter the UI with a separate 'Sort Criteria' for each bookmark.

In my wording here I might have mislead you a bit. Even with my proposed patch the admin would still see a separate sort criteria for each 'mark' in the 'Sort Criteria' box. It's just that with my patch the admin doesn't need to use the 'Sort Criteria' box at all, because the table is click-sortable.

OTOH, hard-coded sort is the only thing needed for a listing showing only usernames --which will happen 95% of the times (untill Views 2 is out, where the admin might will be able to pull in profile fields).

I leave it to you to decide if we want this patch.

If you decide we don't, I can supply an alternative patch, a little one, that would provide hard-coded sort for the 'Sort Criteria' box. It won't require an upgrade path.

I leave it to you to decide if we want this patch.
If you decide we don't, I can supply an alternative patch, a little one, that would provide hard-coded sort for the 'Sort Criteria' box.

Here's this alternative patch. It's the 'trivial' one I initially promised. No upgrade path is needed with this one. And no click-sort. I prefer the previous patch, I think it's more elegant, but it's your decision.

I'd prefer to go with #4,

Thanks! I too think it's the better one.

why start the update at 5150
[...]
but wouldn't it just be 5100?

You're right, it should be '5100'.

It turns out I studied how to name this hook from an old issue (where the '15' would stand for version 1.5 of this module). But the official letter says it should be '5100', as you said. Sorry about that.

I'm attaching the patch again, with this minor fix ('5150' -> '5100').