Forulio - Ruby on Rails based forum engine

Hello,

We are working on Rails based forums engine Forulio. It is easy to use
forum with nice ajax integration.
Take a look at it on http://forulio.com.

Also, I would like to ask for feedback about using Forulio.

Thanks.

Also, I would like to ask for feedback about using Forulio.
Looking good! However reading topics & messages failed for me if I
wasn’t logged in.

Tom C. wrote:

Also, I would like to ask for feedback about using Forulio.
Looking good! However reading topics & messages failed for me if I
wasn’t logged in.

Just fixed. it was after our update

It didn’t remember my timezone when I incorrectly entered my password on
the
sign up page. Checking it out now.

Forget Password should be on another line underneath remember me when
logging in.

I would love to see the code for this.

Ryan B. wrote:

Forget Password should be on another line underneath remember me when
logging in.

I would love to see the code for this.

Thanks for the notes. SVN url is sent.

On 11 Mar 2008, at 13:44, Ryan B. (Radar) wrote:

def last_post
topics.first.posts.last
end

and that’s an at least kind of thing.

Exactly that wouldn’t be a great idea - you wouldn’t want to load all
topics just to get the last post.
On top of this, that doesn’t do what I would expect last_post to do:
it returns the last post in the most recently created topic, not the
last post overall.
A has_one with some conditions is probably the right sort of thing.

Fred

Alright, I know it’s all “beta” and what not, but here’s what I found:
*
forum.rb

1. belongs_to :last_post will not find the last post for the forum. A
forum should not
belongs_to* a last post. To find the last post, you’d
do:

*has_many :topics, :order => “created_at desc”
has_many :posts, :through => “topics”

def last_post
topics.first.posts.last
end*

and that’s an at least kind of thing.

  1. Should description be allowed to be left blank?

*topic.rb

*3. @users_in_topic set randomly out in the middle of nowhere. Just
underneath *validates_presence_of :title.

*4. I don’t see the point of increment_counters and
*decrement_counters. *Isn’t
this what the counter_cache option specified on the associations do?

  1. Why are you defining a method name longer than the actual thing
    you’re
    using to define it with current_forum? You can just reference the
    topic’s
    forum as just *forum *in the model there. So current_forum.topics_count
    +=
    1 if update_self
    becomes just *forum.topics_count += 1 if update_self.

*6. You’re defining @users_in_topic, which simply doesn’t need to be
done.
You can do *has_many :users, :through => :posts *which will do exactly
the
same thing, and you can then reference all the users as just users.

  1. The *last_read_post *definition doesn’t make sense. If you’re
    defining
    this attribute on a topic, it’s not going to be unique for every user. I
    see
    what you’re trying to do, but I don’t think that this will work in it’s
    current incarnation.

  2. What’s the point of the before_destroy now?

user.rb

  1. When saying that the login must not contain invalid characters,
    specify
    which characters they are.

  2. Password Expiry. Why? I know it gives added security if a person is
    changing their password every 30 days, but do you really think forum
    goers
    like doing anything that involves any effort whatsoever, especially
    having
    to think of a new password?

*post.rb

11. Pray tell, why are you using a HELPER in your model? This is
blurring
the lines between the MVC pattern.
*

*read_topic.rb

*I actually think that this is a good idea. I was thinking how to do
this
the other day and this helps.

  1. Why store the last_post? Why not just store the time they last
    accessed
    the topic?

I see good intentions in this project, good luck.

Thanks for review, Ryan!

Ryan B. wrote:

  1. The *last_read_post *definition doesn’t make sense. If you’re
    defining
    this attribute on a topic, it’s not going to be unique for every user. I
    see
    what you’re trying to do, but I don’t think that this will work in it’s
    current incarnation.

method that search for new topics take user as a parameter, so every
search will be user-specific and last_read_post will show last
read_post for given user in each topic.

last_read_post is dynamic attribute of topic, there is no column in db
with this name.

def self.topics_with_last_posts page, user=nil
page ||= 1
joins = " as t left join posts as p on p.topic_id=t.id"
#FIXME: change interval to count days from last visiting date of
current user
conditions = " DATE_SUB(CURDATE(),INTERVAL 90 DAY) <= p.created_at"
select=‘t.*’
if user
joins << " left join read_topics rp on rp.topic_id = t.id and
rp.user_id=#{user.id}"
select << “, rp.last_read_post_id as last_read_post”
end
Topic.paginate(:all, :select=>select, :group => “t.id”,
:joins=>joins, :page => page, :order=>‘t.last_post_id DESC’,
:conditions=>conditions)
end

Thank you for feedbacks and advises.

1. belongs_to :last_post will not find the last post for the forum. A
forum should not
belongs_to* a last post. To find the last post, you’d
do:
We added this for association with last_post_id only - it is setup
manually in the increase/decrease counters functions. I see it is time
to change the names for these functions, as they not only setup counters
now, but also change last_post for topic and forum.

*has_many :topics, :order => “created_at desc”
has_many :posts, :through => “topics”

Thank you, we will look for this, I just never used such syntax before.
:slight_smile:

  1. Should description be allowed to be left blank?
    Why not? In any cases it can be changed in a minute.

*topic.rb

*3. @users_in_topic set randomly out in the middle of nowhere. Just
underneath *validates_presence_of :title.

This is temporary variable that used between before_destroy and
after_destroy.
One of ways to avoid this - is to rewrite destroy method. I think it
will help to avoid this variable.

*4. I don’t see the point of increment_counters and
*decrement_counters. *Isn’t
this what the counter_cache option specified on the associations do?

We’ll rename this - it apply not only counters, but also last post
functionality and apply this caches for parent forum also.

  1. Why are you defining a method name longer than the actual thing
    you’re
    using to define it with current_forum? You can just reference the
    topic’s
    forum as just *forum *in the model there. So current_forum.topics_count
    +=
    1 if update_self
    becomes just *forum.topics_count += 1 if update_self.

Was not sure about using self properties and methods in rails models.
That’s why I defined temporary variable with unique name. We will
rewrite this, if you approach will work correctly in this situation.

*6. You’re defining @users_in_topic, which simply doesn’t need to be
done.
You can do *has_many :users, :through => :posts *which will do exactly
the
same thing, and you can then reference all the users as just users.

Thank you. I think it can help as in future. As I wrote in this case -
this is temporary - I think we will remove it to avoid such
misunderstanding’s.

  1. What’s the point of the before_destroy now?
    Select @users_in_topic - for normalizing counters for users which posts
    was in this topic.

user.rb

  1. When saying that the login must not contain invalid characters,
    specify
    which characters they are.
    Ok.
  1. Password Expiry. Why? I know it gives added security if a person is
    changing their password every 30 days, but do you really think forum
    goers
    like doing anything that involves any effort whatsoever, especially
    having
    to think of a new password?
    Taken from plugin, it not used - so password should not expire. We did’t
    cleanup not used features from authenticated_system.

*post.rb

11. Pray tell, why are you using a HELPER in your model? This is
blurring
the lines between the MVC pattern.
*
MVC - is model view controller. Helpers is rails addition in this case.
I don’t know why this approach is bad.

Ryan B. wrote:

  1. Why store the last_post? Why not just store the time they last
    accessed
    the topic?

Sure, there is no need to store last_post object, as I use last_post_id
property to check is topic new. In this case we only remove belongs_to
:last_post, :class_name=>‘Post’

Oleksandr B. wrote:

Ryan B. wrote:

  1. Why store the last_post? Why not just store the time they last
    accessed
    the topic?

Sure, there is no need to store last_post object, as I use last_post_id
property to check is topic new. In this case we only remove belongs_to
:last_post, :class_name=>‘Post’

After short discussion I see that it is better to store last accessed
time.

Ah Fred thanks for that!

I’m doing my own forum system as a hobby project, and I just copied the
code
directly from that. I didn’t realise what I was doing. It would be nice
if
the topic record’s updated_at was updated when a new post was added to
it.
I’ll have to take a peek at what other solutions for this problem there
are.

I figured it out eventually, it’s just posts.last to get the last post
if
you’ve set it up with this being the bare-minimum:
*
forum.rb
*has_many :topics, :order => “created_at”

alias_method :old_topics, :topics
def topics
old_topics.sort_by { |t| t.posts.last.created_at }.reverse
end

*topic.rb
*has_many :posts

Oleksandr, if you want to see my code I have recently ported it all over
to
git: GitHub - radar/rboard: A fully featured forum system compatible with Rails 2.3

s.ross, that will find the latest topic, not necessarily the the latest
post. For example, I create two topics, Topic A and Topic B. It says
that
Topic B is the newest topic. I post in Topic A, and using your code
Topic B
will still display as the latest topic.

The “cheapest” way I know to find the last post is:

Topic.find(:all, :order => ‘created_at DESC’, :limit => 1)

and make sure there’s an index on created_at.

Of course, I’m lying. The cheapest way is to cache the most recent
post :slight_smile:

:slight_smile:

A forum has many topics, which has many posts, and then you want to find
out
the latest post from every topic, so the post with the latest
created_at. To
do this, you need to find all the posts, which you’re right may be
processor
intensive if there’s a ton of them. I can’t think of an easier way to do
this except store the last_post_id on the forum somewhere, in which case
it’s only a lookup of 1 post, rather than a million.

On Mar 11, 2008, at 9:48 PM, Ryan B. (Radar) wrote:

s.ross, that will find the latest topic, not necessarily the the
latest post. For example, I create two topics, Topic A and Topic B.
It says that Topic B is the newest topic. I post in Topic A, and
using your code Topic B will still display as the latest topic.

I guess I’m missing the definition of “latest,” or perhaps of “post.”
In any case, doing a full-table scan and then picking the post out by
sorting then reversing the whole row-set in Ruby will normally be
slower than letting the database do that part of the work.

My point is that the database will approach O(n log m) and incur far
less object-creation overhead whereas doing a reverse in Ruby is at
best O(n) over and above the sort which is probably O(n log m).

Just a thought…

On 12 Mar 2008, at 12:02, Ryan B. (Radar) wrote:

A forum has many topics, which has many posts, and then you want to
find out the latest post from every topic, so the post with the
latest created_at. To do this, you need to find all the posts, which
you’re right may be processor intensive if there’s a ton of them. I
can’t think of an easier way to do this except store the
last_post_id on the forum somewhere, in which case it’s only a
lookup of 1 post, rather than a million.

If you’ve got the right indexes, topic.posts.find(:first, :order =>
‘something desc’) (or Post.find(:first, :order => ‘something desc’)
if you don’t want it scoped) is lightning fast.

Fred