Skip to content

Commit

Permalink
Merge branch 'fix_issue_with_group_by'
Browse files Browse the repository at this point in the history
  • Loading branch information
jhirbour committed Jan 9, 2020
2 parents 309383c + 0358280 commit 696086a
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 2 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# What's new

### 1.0.2 (January 9th, 2020)
* This version fixes a bug with FrontEndBuilds::AppsController#index where it would not show the `/frontends`.
- This bug was introduced in 1.0.0 (rails 5 updates).
- The controller should return "10 builds for each app", instead it was
returning "10 builds for all apps". This and issue when one of your apps has
a really old "live build" that is older than your 10 most recent (for any app)

### 1.0.1 (May 6th, 2019)
* `FrontEndBuilds::App.live_build` is now optional. This resolves issues with Rails 5 clients that have `Rails.application.config.active_record.belongs_to_required_by_default` enabled.

Expand Down
8 changes: 7 additions & 1 deletion app/controllers/front_end_builds/apps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ class AppsController < ApplicationController
before_action :set_app , :only => [:show, :destroy, :update]

def index
apps = App.includes(:recent_builds)
# this should be the most recent 10 builds for each app

# DO NOT use `App.includes(:recent_builds)`
# b/c it mucks up the grouping logic and only gives the most
# recent 10 for ALL apps not 10 per app
apps = App.all

respond_with_json({
apps: apps.map(&:serialize),
builds: apps.map(&:recent_builds)
Expand Down
4 changes: 4 additions & 0 deletions front_end_builds.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Gem::Specification.new do |s|
s.add_development_dependency 'rubocop'
s.add_development_dependency 'rubocop-rspec'
s.add_development_dependency 'shoulda-matchers', '2.7.0'
# These 2 are needed so that the rails app version matches
# otherwise bundle gives you sprockets 4
s.add_development_dependency 'sprockets', '3.7.2'
s.add_development_dependency 'sprockets-rails', '3.2.1'
s.add_development_dependency "sqlite3"
s.add_development_dependency 'webmock'
end
2 changes: 1 addition & 1 deletion lib/front_end_builds/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module FrontEndBuilds
VERSION = "1.0.1"
VERSION = "1.0.2"
end
30 changes: 30 additions & 0 deletions spec/controllers/front_end_builds/apps_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,36 @@ module FrontEndBuilds
expect(json['apps'].length).to eq(1)
expect(json['builds'].length).to eq(3)
end

# This specs query composition b/c it changed slightly between rails 4 and rails 5
# in regards to includes
describe "testing query composition", focus: true do
# getting rid of the ones from the outer describe
before(:each) do
App.delete_all
Build.delete_all
end

let(:app1) { create(:front_end_builds_app, name: 'dummy') }
let(:app2) { create(:front_end_builds_app, name: 'dummy2') }
let!(:app1_builds) { create_list(:front_end_builds_build, 1, app: app1) }
let!(:app2_builds) { create_list(:front_end_builds_build, 10, app: app2) }

it "Finds the correct builds_ids for EACH app" do
get :index, format: :json

expect(response).to be_success
expect(json['apps'].length).to eq(2)
app1_json = json['apps'].select{|x| x['id'] == app1.id}.first
app2_json = json['apps'].select{|x| x['id'] == app2.id}.first

# make sure the oldest app (by created_by) shows up
# this rows ends up missing if include(:recent_builds) is in the Arel
expect(app1_json['build_ids']).to match(app1_builds.map(&:id))

expect(app2_json['build_ids']).to match( app2.recent_builds.map(&:id))
end
end
end

describe 'show' do
Expand Down

0 comments on commit 696086a

Please sign in to comment.