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 Campbell 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.

Ryan Bigg 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.

Alright, I know it’s all “beta” and what not, but here’s what I found:
forum.rb1. 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.
2. Should description be allowed to be left blank?
topic.rb3. @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?
5. 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.
7. 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.
8. What’s the point of the before_destroy now?
user.rb
9. When saying that the login must not contain invalid characters, specify which characters they are.
10. 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.rb11. Pray tell, why are you using a HELPER in your model? This is blurring the lines between the MVC pattern.
read_topic.rbI 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.

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.

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

Thanks for review, Ryan!

Ryan Bigg wrote:

7. 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:

2. 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.

5. 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.

8. 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*

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

Ok.

10. 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 Bigg wrote:

12. 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 Bondar wrote:

Ryan Bigg wrote:

12. 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.

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.rbhas_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.rbhas_many :posts

Oleksandr, if you want to see my code I have recently ported it all over to git: http://github.com/Radar/rboard/tree/master

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:

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...

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

I haven’t defined any indexes on any of the tables, and I probably should.

Lightning fast you say? This is interesting.

Man, that last_post caching was probably such a bad hack feels ashamed

I haven't defined any indexes on any of the tables, and I probably
should.

Lightning fast you say? This is interesting.

Yes - if you've got an index on a column then finding the first/last n
values in the table, ordered by that column is fast because the
database can use its index
In the second case (find the last post in topic x) then if you have an
index on [topic_id, created_at] then finding the first/last n values
order by created_at where topic_id is some fixed value is also fast
I mostly know about mysql but mysql isn't usually smarter than
postgres etc... so I would had thought most dbs would be good at this.

Fred.

if your database supports subselects (like PostgreSQL), you could do a
find_by_sql with sql like this:

SELECT DISTINCT t.*, last_post
FROM topics t
  JOIN (SELECT topic_id, MAX(created_at) FROM posts GROUP BY topic_id)
as foo
    ON (foo.topic_id = t.id)
ORDER BY last_post DESC

This will involve a full table scan on posts and topics, but should
work well until you have a very large database. It will always be
faster than doing the equivalent processing in ruby/rails using
ActiveRecord objects.

However, when your forum became very large and successful, you'd have
to optimize this by storing last_post_id, updating when posts are
created, and set up

belongs_to :last_post, :class_name => 'Post', :foreign_key
=> :last_post_id

then in a single

Topic.find(:all, :include => [:last_post], :order => "post.created_at
DESC")

you could load the Topics, in descending order by last post, with the
last post already instantiated so you could show those details in your
listing, and it would be easy to add the appropriate limit and offset
parameters for paging. That's what I would do, if I was writing forum
software...

Jim Crate
Que Viva, LLC