Update to Rails 5 #3051
Update to Rails 5 #3051
Conversation
…fficial repo and upgraded to 0.15
… cleaner strategy for feature specs. Updates syntax for user follows feature specs to match general style.
…for recently updated query object.
…ails-controller-testing gem.
There was a problem hiding this comment.
I was going to use the contents of the coverage
directory to confirm what percentage of the codebase in general. In addition, would it be fair to say that we have 38 controller actions (I found that using cd app/controllers; find . -name "*_controller.rb" | xargs grep -E "def (index|create|update|new|show|edit|destroy)" | wc -l) and 21 request specs (cd spec/requests ; find . -name "*.rb" | xargs grep -E " describe '" | wc -l) or 55% request spec coverage now?
| // font-size: small; | ||
| color: white; | ||
| background-color: $color-gray; | ||
| background-color: $color-orange; |
There was a problem hiding this comment.
@rlgreen91 Is this a consolidation of CSS files? I feel like it may be helpful to separate this into another PR in order to keep things manageable.
There was a problem hiding this comment.
This is part of the situation I outlined in the comment below.
| <% if user_signed_in? %> | ||
| <% if current_user.following?(@this_case) %> | ||
| <%= link_to 'Unfollow', case_follow_path(@this_case, @follow_id), :confirm => "Are you sure you want to unfollow this case?", :method => :delete, class: "btn btn-primary.outline", id: "unfollow-button" %> | ||
| <%= button_to 'Unfollow', follows_case_path(@this_case), method: :delete, data: { confirm: "Are you sure you want to unfollow this case?" }, form_class: "btn btn-primary btn-lg", id: "unfollow-button" %> |
There was a problem hiding this comment.
@rlgreen91 button_to has been around since 4.2. Is this an essential piece of upgrading to 5.0, or is this a separate kind of upgrade?
There was a problem hiding this comment.
Part of upgrading. Some of the changes I made ended up affecting how we handled the path for following cases. That, in turn, led me to making the change to the path here, where I realized that eh, technically we shouldn't be using a link styled as a button to perform a post - it should actually be a button. So I changed that here as part of it since I was already there.
Ironically enough, it was trying to fix the styling of the "unfollow" button that prompted my idea about the CSS fix. The other two were fairly straightforward to change.
|
I'm not sure why I can't respond directly to the spec comment, but I'll do so here. First, I think 21 request specs for 38 controller actions is about 55% yes, but keep in mind that we do have some non-rest controller actions so that number is probably lower. Ideally, I'd like to get to 90+% coverage of controller actions, which would account for static pages that don't really need to be tested. However, as we saw with the conversations request spec, adding them for some controller actions leads to issues and bug fixes that are more complicated than expected and most likely out of scope of this PR. I would like to keep adding request specs, of course, but honestly, I think what we have here is fine as far as upgrading to 5 is concerned. Given that our previous controller tests didn't cover everything and were often testing only a single happy path, I don't think we've lost much coverage in terms of functionality. |
|
@rlgreen91 I'd do one |
|
As per @trystant's message in Slack, this PR has been approved. |
First update to Rails 5