FIX: move data to separate tables (PR #52)

We are trying to avoid custom tables. Changes: CategoryCustomField -> DiscourseVoting::CategorySetting # contains infromation if voting is enabled for category UserCustomField -> DiscourseVoting::Vote # user’s votes TopicCustomField -> DiscourseVoting::VoteCounter # cache count for topics

GitHub

Do we need to delete this in a post migration instead? Otherwise, voting will be disabled before the new code has been deployed? Another suggestion I have is to not delete the custom field first so that if the migration goes wrong, we have a way to reconcile the results.

    raise ActiveRecord::IrreversibleMigration

Same comment as https://github.com/discourse/discourse-voting/pull/52/files#r468979055

  raise ActiveRecord::IrreversibleMigration

Same comment as https://github.com/discourse/discourse-voting/pull/52/files#r468979055

discourse_voting_vote_counters doesn’t seem clear to me as to what data the table holds. Perhaps discourse_voting_topic_vote_count?

We can then rename this to count

I’m also wondering if this table is necessary since we can calculate the vote counts from the discourse_voting_votes table.

  raise ActiveRecord::IrreversibleMigration

yeah, I wasn’t sure about that, because someone can wrongly assume it is active record count method. Do you still vote for count? Just want to double-check :slight_smile:

Yeah fair point, I think it might be useful because that cache would speed up topics page (we will not need to join votes and count). Before that table, it was using topics_custom_field to collect count, so I was thinking that it makes sense to continue

Maybe votes_count? Btw am I right to say that this acts as a counter cache for discourse_voting_votes?

lol I just realized if we name it votes_count it’ll be topic.vote_count.votes_count.

Should we destroy the votes and vote_counter records if the topic is destroyed?

I wonder if we need to do a COUNT(*) here when there is a unique index on topic_id and user_id on the discourse_voting_votes table. I think checking for existence should be sufficient?

I guess it’ll be the same as SELECT 1 AS current_user_voted… so no big deal here. I would prefer if the value is a boolean instead though so we don’t have to do current_user_voted > 0

Since we now have a TopicExtension for votes, maybe all these voting related methods should fall under there as well?

Should we use the count from discourse_voting_vote_counter instead?

We don’t need this set anymore since there is a unique index on the catgegory_id column. That does raise an interesting question though, do we have duplicated records that might cause the add unique index to fail?