Feature/logic out of templates #74
Conversation
| @@ -2,7 +2,7 @@ | |||
| <span class="left"><%= link_to( category.name, category_path(category.id) ) %></span> | |||
| <p class="right"> | |||
| <ul> | |||
| <%= render :partial => 'home/article', :collection => category.articles.by_access_count.limit(3) %> | |||
| <%= render :partial => 'home/article', :collection => category.articles[1..3] %> | |||
There was a problem hiding this comment.
Why not use by access count and use .limit(3)? Currently it's taking articles 2-4.
There was a problem hiding this comment.
Good catch. I guess I was biased against having AR methods in the view--but we already have Category#by_access_count in there so it probably won't hurt to keep #limit.
FIXED
Sent from my iPhone
On Jun 9, 2014, at 4:35 AM, Justin Grevich [email protected] wrote:
In app/views/home/_category.html.erb:
@@ -2,7 +2,7 @@
<%= link_to( category.name, category_path(category.id) ) %>
- <%= render :partial => 'home/article', :collection => category.articles.by_access_count.limit(3) %> - <%= render :partial => 'home/article', :collection => category.articles[1..3] %> Why not use by access count and use .limit(3)? Currently it's taking articles 2-4.
—
Reply to this email directly or view it on GitHub.
|
Good to go, @pui. I want to hear back from @hale re codeforamerica/honolulu_answers#60 and circle back for the CSS stuff. Until then, everything here is ready to merge. |
|
Per codeforamerica/honolulu_answers#60, I will pull out |
|
Please rebase. |
This adds basic test coverage (happy path and other simple examples) for the controllers. It also removes a bunch of unused code where appropriate.
|
rebased |
|
Hi, thanks for this. It's going to be a few days before we can look at it again! |
[cleanup] Fixes typo in spec declaration
Conflicts: spec/models/article_spec.rb
There is a bug in the category#index view since it is expecting an article object to exist. It seems it was intended to only display categories with published articles so I added the code needed for that (and tests). I also moved dotenv-rails in the Gemfile so that it is included in the production env.
There is a bug in the category#index view since it is expecting an article object to exist. It seems it was intended to only display categories with published articles so I added the code needed for that (and tests). I also moved dotenv-rails in the Gemfile so that it is included in the production env.
There is a bug in the category#index view since it is expecting an article object to exist. It seems it was intended to only display categories with published articles so I added the code needed for that (and tests). I also moved dotenv-rails in the Gemfile so that it is included in the production env.
|
Was this PR ever addressed/merged? I'm seeing a failing Travis build and looks like the branch has been dead for sometime now, but there's still quite a bit of value here. Wondering if anyone wants to pick it up again? |
Still a few more templates to fix, WIP.