Finished homework

[oscarlopezalegre]

@coderschoolreview

Find the homework 3 updated.

Please help me to review it shortly as I can't create more instances of heroku

Regards, Oscar

[coderschoolreview]

Thank you for your Ruby submission, oscarlopezalegre!

:x: Unfortunately there are some problems with your submission:

• I couldn't find a walkthrough GIF referenced from your README.md. Please use RecordIt or LICEcap to record an animated GIF of your app in action, and include it in your README.md.

[oscarlopezalegre]

https://giphy.com/gifs/26tPprwWQrZXQXUXe

Find it here!

[harley]

Hi @oscarlopezalegre :+1: good job with the homework. The goal this week was to enhance a basic app with search, complex form, relationships between models and add tests on top of that.

I see that you attempted to use the nested_form gem. This gem isn't actively maintained so there may be rough edges. Did you have any trouble with it?

Overall, the functionalities are good! I just few a few suggestions:

• Code style: I recommend taking a quick read of the ruby style guide and revisit it often. There are inconsistencies in the formatting of your code currently, but this is easy to fix.
  • Don't use tab characters in your code. Use 2 spaces for each tab instead (E.g. in EventsController, index action, the spacing is off)
  • By convention, use session[:user_id] instead of session[:userid]
  • Put spaces around operators. E.g. @editing_event.published=params[:publish] should be @editing_event.published = params[:publish]
• Similar to our notes in the last assignment, please try to avoid too much logic in the view files. As much as possible, only use <% %> and <%= %> for basic blocks, loops and value output.
  • Consider moving the logic of event_in_past and check_ticket_error! from PurchaseController to Event model, such as @event.already_passed? and @event.still_has_tickets?
• Use RESTful routes when possible: consider using resources :purchases instead of purchase routes
• You use .col-sm-2 class for your div.signup-form on users/new: this is too small for small screen sizes.
• Back to the topic of avoiding logical code blocks in the view, let's look at a way to refactor the code in your tickets/new.html.erb:

# avoid this
<select name=<%= "type_id_"+type.id.to_s%>>
  <% 
    if type.max_quantity != nil and type.max_quantity < 11
        @max_quantity= type.max_quantity
    else
        @max_quantity = 11
    end
    @max_quantity.times do |i| %>
    <option> <%= i %> </option>
    </li>
  <% end %> 
</select>

Consider this instead:

# in app/models/event_type.rb
class EventType < ActiveRecord::Base
  MAX_QUANTITY_DEFAULT = 11

  # ...
  def max_quantity_or_default
    max_quality || MAX_QUANTITY_DEFAULT
  end
  # ...
end

# in app/views/tickets/new.html.erb
<select name=<%= "type_id_#{type.id}" %>
  <% @type.max_quantity_or_default.times do |qty| %>
    <option><%= qty %></option>
  <% end %>
</select>

It's even cleaner if you want to use Rails' helper:

<%= select_tag "type_id_#{type.id}", options_for_select((1..@type.max_quantity_or_default).to_a) %>

An alternative approach: you can also set a database default value for the max_quantity column to be 11!

• In events#update, you don't have to call .save again after calling .update. @editing_event.update is the same as @editing_event.update_attributes but the latter is a little more commonly used

def update
  @editing_event = Event.find(params[:id])
  if @editing_event.update_attributes(event_params)
    redirect_to myevents_path, flash: {success: "Message saved"}
  else
    redirect_to edit_event_path(@editing_event), flash: {warning: "Couldn't edit message")
  end
end

• A few layout issues when the screen size is small (check out the bootstrap documentation on ways to avoid this) @coderschoolreview @chug2k

[oscarlopezalegre]

nesteed_form was working well for my app, My guess is that it was just a little of javascript library. It was important to have a single page for the users. In the future I might use it again.