vasconsaurus / produce_in_season

0 stars 0 forks source link

feat: choose a month and view it's produce #32

Closed vasconsaurus closed 2 years ago

vasconsaurus commented 2 years ago

closes #29

I tried to start by writing tests, but the logic I thought at first didn't work for the feature. I wrote as if we were creating seasons in the index, but we are only pulling our 12 months from our dictionary and redirecting to the month with it's produce.

As I moved foward and wrote more tests, I realized something was off, as they kept failling, and I couldn't write them properly. The problem was I was trying to pass month_index as a param, but using id. So I changed our item_seasons to use the month_index as param. I also added a helper to get the month index from the month name.

That worked, but I wrote: @item_season = ItemSeason.find_by_month_index(params[:month_index]) Which the rubocop keeps yelling about 😬 , when I change to find_by, as it suggests, it breaks

not directly realted to this feature

related to testing

I'm still not too sure about the tests. Could you point out which ones you think are usefull, which aren't, and some ways I could make them better?

tcione commented 2 years ago

@tcione

This change was just a result of me messing around trying to figure this out. Since I changed the logic to the controller and changed the instance variable name both #show tests broke. I'm getting this error:

     Failure/Error: <% @produce_item.find_each do |produce| %>

     ActionView::Template::Error:
       undefined method `find_each' for nil:NilClass

So I'm guessing my problem is with how I'm creating the variables. I'll keep poking around. But could you suggest maybe what would be a good way to debug rspec?

This is the a problem similar to the one you faced in a previous PR. You are assigning an array in a place in which you actually expect an ActiveRecord relation. The correct thing would be assigning ItemSeason.(where|all) instead.

vasconsaurus commented 2 years ago

This is the a problem similar to the one you faced in a previous PR. You are assigning an array in a place in which you actually expect an ActiveRecord relation. The correct thing would be assigning ItemSeason.(where|all) instead.

Yeah, I figure that out, buuuut was still having problems with it 🥲. What seems to have fixed it is using: :produce_item, temSeason.includes([:produce_item]).where(month_index: item_season.month_index)

Still, tips on debugging rspec would be greatly appreciated 😆 I like the options of debugging with capybara, but I still need to learn better ways to debug rspec.

tcione commented 2 years ago

@vasconsaurus

Yeah, I figure that out, buuuut was still having problems with it 🥲. What seems to have fixed it is using: :produce_item, temSeason.includes([:produce_item]).where(month_index: item_season.month_index)

That was the suggestion. You should always assign data as close as possible to the actual application. That means respecting data type (ActiveRecord relation VS array), filtering (where(...)), etc.

As for debugging, there are two ways:

  1. Adding prints (p, puts, etc) in places to check whatever is there at a certain point
  2. Using a debuggin tool like byebug or pry.

That said, I'm not sure what do you mean when you mention debugging capybara. You mean seeing the browser at work?


Then as a final observation (and a nitpick), capybara runs using RSpec in this project, so putting them on opposite ends might be misleading. You could probably get around this by saying things like "pure rspec tests", integration tests, view tests, etc.

vasconsaurus commented 2 years ago

That said, I'm not sure what do you mean when you mention debugging capybara. You mean seeing the browser at work?

I mean that when I looked at the docs for capybara there were a few debugging tools, like save_and_open_page. That actually helped me quite a bit. With the other parts of rspec I didn't find something like that. I didn't mean to separate them though.

I read a bit about using pry with rspec, I think I'm going to probably try that or byebug

tcione commented 2 years ago

More as an FYI, since I'm changing the comment style. We are currently adopting Conventional Comments in my team, so I'm using this project to get used to it as well :p