Hi there, When a user has no blog posts, a simple message should be present, otherwise it looks like the system encountered an error, heres my simple patch, it will apply for 4.x and 5.x


--- blog.module.cvs 2007-04-26 14:53:23.000000000 +1000
+++ blog.module 2007-04-26 14:54:39.000000000 +1000
@@ -148,9 +148,14 @@
}

$result = pager_query(db_rewrite_sql("SELECT n.nid, n.sticky, n.created FROM {node} n WHERE n.type = 'blog' AND n.uid = %d AND n.status = 1 ORDER BY n.sticky DESC, n.created DESC"), variable_get('default_nodes_main', 10), 0, NULL, $account->uid);
- while ($node = db_fetch_object($result)) {
- $output .= node_view(node_load($node->nid), 1);
+ if ( db_num_rows($result) > 0 ) {
+ while ($node = db_fetch_object($result)) {
+ $output .= node_view(node_load($node->nid), 1);
+ }
+ } else {
+ $output .= t('%name has not yet made any blog posts.', array ('%name' => ucwords($account->name)) );
}
+
$output .= theme('pager', NULL, variable_get('default_nodes_main', 10));
drupal_add_feed(url('blog/'. $account->uid .'/feed'), t('RSS - !title', array('!title' => $title)));

Comments

StevenPatz’s picture

Status:Reviewed & tested by the community» Needs review
ChrisKennedy’s picture

Status:Needs review» Needs work

This looks like a good idea; I agree that the current blank page is confusing. The patch needs a few tweaks though:

  • Coding style needs work - too many spaces inside of parentheticals and the if/else brace style is wrong. See http://drupal.org/node/318
  • The div tags should be outside of the translated text and concatenated.
  • The username should not have ucwords applied to it - it should be displayed with its original capitalization.
dgtlmoon’s picture

StatusFileSize
new1 KB

Hi there, well spotted, i have supplied a new patch

cheers

ChrisKennedy’s picture

Closer, but the "not yet made any blog posts" line still has coding style errors due to the extra spacing. Also that second extra hunk needs to be removed.

msameer’s picture

Version:5.x-dev» 6.x-dev
Status:Needs work» Needs review
StatusFileSize
new1.07 KB

Fixed the style and removed the extra hunk.

ChrisKennedy’s picture

StatusFileSize
new1.2 KB

That version still had several errors... here's an updated patch.

Junyor’s picture

Is there any reason you're using a DIV instead of a P? A P element would be more semantically correct.

ChrisKennedy’s picture

StatusFileSize
new1.2 KB

Paragraph it is.

Freso’s picture

Is there a reason for not using theme_username()? I'm thinking array('%name' => theme_username($account)) would be good, assuming $account is a user object. :)

Freso’s picture

StatusFileSize
new1.21 KB

Patch doesn't apply:

gentoo-vm drupal6 # patch -p0 < blog_noposts_0.patch
patching file modules/blog/blog.module
Hunk #1 FAILED at 151.
1 out of 1 hunk FAILED -- saving rejects to file modules/blog/blog.module.rej

So I re-rolled. I'm having problems with blog.module though - neither this patch nor the one at issue 179519 work for me. :(

Gábor Hojtsy’s picture

Hm, reusing theme_username() seems to be a good idea indeed. But that might spit out HTML, so it should be checked, whether it escapes the name properly, so we can use !name.

bennybobw’s picture

theme_username spits out escaped html e.g.

&lt;a href=&quot;/user/1&quot;&gt;admin&lt;/a&gt;

note that a blank page comes up also with /blog
I think this patch should be expanded to include that page as well.

RobLoach’s picture

Status:Needs review» Needs work

I had troubles applying this patch, and db_num_rows() was removed.

RobLoach’s picture

Status:Needs work» Needs review
StatusFileSize
new1.36 KB

Did a couple things here...

  • Replaced db_num_rows() as instructed
  • Themed the username when telling the user that they have no blog posts
  • Display "You" instead of their user name if they're looking at their own blog
  • Use drupal_set_message instead of just outputting the message
catch’s picture

Status:Needs review» Needs work

Empty blogs by other users now generate page not found (which seems sensible to me) so the else statement is a bit redundant. I'd support the "You have not made any blog posts" message for global $user though.

RobLoach’s picture

There are four use cases with the patch I uploaded...

  1. Visiting a blog for users who do not have permission to have a blog (edit own blog permission), you get "Access denied". This is managed by hook_menu checking user_access with the user.
  2. When visiting a blog for a user that doesn't exist (blog/999999 for example), you get a "Page not found", because the user doesn't exist.
  3. When visiting a blog for users who have permission to have a blog, but haven't posted any items, you get "%user's blog", with a message saying "%user has not made any posts".
  4. When visiting your own blog, and you have the edit own blog permission, and haven't posted any blog posts, you get a page saying "You have not made any blog posts."

The redundant else you were talking about there is why we're talking about this. We want to display a message when a user doesn't have any blog posts, and this patch does just that, that else does that. I don't think there's anything missing from this patch...

matt@antinomia’s picture

StatusFileSize
new2.31 KB

Rob's patch works for me. The attached patch adds similar functionality to the global blog page if there are no posts. A new user to Drupal would likely find this more helpful than a blank screen.

RobLoach’s picture

Status:Needs work» Needs review

Ahh, sounds good!

birdmanx35’s picture

Version:6.x-dev» 7.x-dev

This is a feature request currently, and it adds strings, so I am moving this to 7.x-dev.

RobLoach’s picture

Although it is kind of a feature request, it doesn't break any strings. Adding strings is okay, changing them is not.

birdmanx35’s picture

Ah, okay, thanks Rob.

dgtlmoon’s picture

Version:7.x-dev» 6.x-dev

Yes it adds a string but what does it matter, it really cleans up the experience in an a module that goes way back in drupal's development history

RobLoach’s picture

How to test:

  1. Fresh Drupal install
  2. Visit blog/1... See your name and "Post new blog entry"
  3. Visit blog... And see an empty page saying "Blogs"
  4. Create a new user
  5. Visit blog/#, where # is the new user's ID and see Access Denied
  6. Apply patch
  7. Visit blog/1... See your name, "You have no made any blog posts.", and "Post new blog entry"
  8. Visit blog... And see "No one has made any blog posts"
  9. Log out and visit blog/1 and see "[User] has not made any blog posts"
  10. Set to RTBC
keith.smith’s picture

Status:Needs review» Needs work

Just on a visual review of the patch, we've been referring to blog posts as "blog entries". And, while I like "posted" personally, we have "Create content", and usually use "post" as a noun rather than verb. (Although that isn't consistent, as the button on this comment page says "Post comment".) So, the strings, if they were to get in post-string-freeze, would need to say something like:

+      drupal_set_message(t('You have not created any blog entries.'), 'status');
...
+      drupal_set_message(t('!author has not created any blog entries.', array('!author' => theme('username', $account))), 'status');
...
+    $items[] = l(t('Create new blog entry.'), "node/add/blog");
...
+    drupal_set_message(t('No blog entries have been created.'), 'status');
keith.smith’s picture

My apologies -- on reflection and a bit of grepping, I was quite wrong on my assessment of the use of "Post". I count five instances of using "Post" as a piece of content, and four using "Post" as the act of creating the content. And, more importantly, blog module does say "Post new blog entry" and not "Create new blog entry".

RobLoach’s picture

I think we should change those strings. The blog module has been around since the beginning, and needs as much cleaning as it can get.

dgtlmoon’s picture

+1 @ rob loach :)

Pancho’s picture

Category:feature» bug

This is not a new feature, it's a bug indeed. Needs some more eyes on it, though.

RobLoach’s picture

Status:Needs work» Needs review
StatusFileSize
new2.28 KB

This patch features the code changes in #17, as well as Keith's suggested string fixes in #24.

GreenLED’s picture

Version:7.x-dev» 6.x-dev
Status:Closed (fixed)» Postponed (maintainer needs more info)

Ran the patch.

Before:
1. Clicked through to "/blog/1"

After:
1. Message appeared,

admin has not created any blog entries.

As expected.

At this quick glance, the patch looks like it's ready to me.
Are there any other pages, etc. that are affected by this
patch or is this the only one?

» Respectfully, GreenLED
» Stable Files . net

Junyor’s picture

catch’s picture

GreenLED: only /blog - otherwise feel free to mark RTBC if you think it fixes the issue appropriately.

cburschka’s picture

Status:Needs review» Reviewed & tested by the community

Patch applies to DRUPAL-6. All three messages (global blog, your own blog, someone else's blog) work fine. I believe this makes two reviews.

(PS: Rob, the -p switch on diff is useful in displaying the header of the function a change was made in. Unlike -u, I'm not sure it's required practice, but it's very helpful.)

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Needs work

- 'status' is the default type, so we do not specify that in other places.
- instead of $num_rows, we should use $num_posts or something like that. I understand we are not displaying all posts, so maybe $num_displayed or something like that sounds better.

cburschka’s picture

Status:Needs work» Needs review
StatusFileSize
new2.33 KB

- 'status' removed
- $num_rows changed to $has_posts, as it is a boolean that only becomes TRUE if there is at least one post.

I tested this, but of course another review couldn't hurt.

Gábor Hojtsy’s picture

Version:6.x-dev» 7.x-dev
Status:Needs review» Reviewed & tested by the community

Since the previous version was well tested and this is only a coding style change, I committed #35. Thanks. RTBC for 7.x.

GreenLED’s picture

Status:Postponed (maintainer needs more info)» Reviewed & tested by the community

Adjustments:
1. Clicked through to "/blog/"
2. Message appears as expected.

No blog entries have been created.

3. Successfully tested using clean url on.
4. Successfully tested using clean url off.

It looks like we're ready to add this.

» Respectfully, GreenLED
» Stable Files . net

Gábor Hojtsy’s picture

GreenLED’s picture

Which patch is for 6.x, dev?

blog_noposts-29.patch

or

blog_no_posts-139290-35.patch

» Respectfully, GreenLED
» Stable Files . net

catch’s picture

#35 - blog_no_posts-139290-35.patch

Dries’s picture

Committed to CVS HEAD. Thanks.

Dries’s picture

Status:Reviewed & tested by the community» Fixed
dgtlmoon’s picture

Version:7.x-dev» 6.x-dev

@dries: so this is HEAD, this bug should be 6.x ?

Junyor’s picture

@dgtlmoon: Please read the full issue. A patch was committed to the DRUPAL-6 branch on February 8th.

Junyor’s picture

Version:6.x-dev» 7.x-dev
dgtlmoon’s picture

Gábor Hojtsy updated the bug to 'ready to be comitted' but said he committed it in the comment, i guess i oversighted his comment somewhere in the 46 posts..

RobLoach’s picture

Anonymous’s picture

Status:Fixed» Closed (fixed)

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

Status:Reviewed & tested by the community» Closed (fixed)