Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

case-insensitive gem search #107

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ storage
tmp

public/sitemap.xml.gz

app/assets/builds
public/assets
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were generated locally but not ignored. I assume they should be.

5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ In order to set up the application locally:

1. `git clone git@github.com:railsbump/app.git`
2. `bin/setup`
3. `foreman start -f Procfile.dev`
4. Go to http://localhost:3000
3. `rake data:find_or_create_rails_releases`
4. `foreman start -f Procfile.dev`
5. Go to http://localhost:3000
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed this extra step to get the app up and running


If these steps don't work, please submit a new issue: https://github.com/railsbump/app/issues/new

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/releases_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def create
if name == "rails"
RailsReleases::Create.perform_async params.fetch(:version)
else
Gemmy.find_by(name: name)&.then {
Gemmy.find_by_name(name)&.then {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a global search and replace for find_by!?(name:...

Gemmies::Process.perform_async _1.id
}
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/gemmies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def index
end

def show
@gemmy = Gemmy.find_by!(name: params[:id])
@gemmy = Gemmy.find_by_name!(params[:id])
end

def compat_table
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/rails_releases_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class RailsReleasesController < ApplicationController
def show
@gemmy = Gemmy.find_by!(name: params[:gemmy_id])
@gemmy = Gemmy.find_by_name!(params[:gemmy_id])
@rails_release = RailsRelease.find_by!(version: params[:id].gsub("rails-", "").gsub("-", "."))
end
end
end
11 changes: 11 additions & 0 deletions app/models/gemmy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ class Gemmy < ApplicationRecord

delegate :to_param, :to_s, to: :name

# Find existing by case-insensitive name
def self.find_by_name(name, raise_error: false)
find_by!("LOWER(name) = ?", name.downcase)
rescue ActiveRecord::RecordNotFound => e
raise e if raise_error
end

def self.find_by_name!(name)
find_by_name(name, raise_error: true)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and added these two methods for case-insensitive search.

The other option of course would be to change the data type to citext, which requires a postgres extension. I think that would be cleaner but didn't want to go quite that far.


# Check all pending compats for compatibility
def check_compatibility
compats.pending.each do |compat|
Expand Down
2 changes: 1 addition & 1 deletion app/models/lockfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def add_gemmies
return if gemmies.any?

gem_names.each do |gem_name|
gemmy = Gemmy.find_by(name: gem_name) || Gemmies::Create.call(gem_name)
gemmy = Gemmy.find_by_name(gem_name) || Gemmies::Create.call(gem_name)
self.gemmies << gemmy
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/gemmies/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def call(name)
raise Error, "Please enter a name."
end

if existing_gemmy = Gemmy.find_by(name: name)
if existing_gemmy = Gemmy.find_by_name(name)
raise AlreadyExists.new(existing_gemmy)
end

Expand Down
66 changes: 34 additions & 32 deletions config/sitemap.rb
Original file line number Diff line number Diff line change
@@ -1,40 +1,42 @@
require "fog-aws"
unless Rails.env.local?
Copy link
Contributor Author

@trusche trusche Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit brute force, but loading this caused errors locally that can be ignored, I think, if we don't want to connect to AWS in development. I can take it out if you like, but this prevents local dev from running without AWS keys. The only change is the guard clause, the rest is whitespace.

require "fog-aws"

SitemapGenerator::Sitemap.default_host = "https://www.railsbump.org"
SitemapGenerator::Sitemap.public_path = "tmp/sitemaps/" # Temporary storage before uploading to S3
SitemapGenerator::Sitemap.adapter = SitemapGenerator::S3Adapter.new(
fog_provider: "AWS",
fog_directory: "railsbump.org",
fog_region: ENV["AWS_REGION"],
aws_bucket: "railsbump.org",
aws_access_key_id: ENV["AWS_ACCESS_KEY_ID"],
aws_secret_access_key: ENV["AWS_SECRET_ACCESS_KEY"],
aws_region: ENV["AWS_REGION"]
)
SitemapGenerator::Sitemap.default_host = "https://www.railsbump.org"
SitemapGenerator::Sitemap.public_path = "tmp/sitemaps/" # Temporary storage before uploading to S3
SitemapGenerator::Sitemap.adapter = SitemapGenerator::S3Adapter.new(
fog_provider: "AWS",
fog_directory: "railsbump.org",
fog_region: ENV["AWS_REGION"],
aws_bucket: "railsbump.org",
aws_access_key_id: ENV["AWS_ACCESS_KEY_ID"],
aws_secret_access_key: ENV["AWS_SECRET_ACCESS_KEY"],
aws_region: ENV["AWS_REGION"]
)

opts = {
create_index: true,
default_host: "https://www.railsbump.org",
compress: false,
public_path: "/tmp",
sitemaps_host: ENV["FOG_URL"],
sitemaps_path: ""
}
opts = {
create_index: true,
default_host: "https://www.railsbump.org",
compress: false,
public_path: "/tmp",
sitemaps_host: ENV["FOG_URL"],
sitemaps_path: ""
}

rails_releases = RailsRelease.order(:version).to_a
rails_releases = RailsRelease.order(:version).to_a

SitemapGenerator::Sitemap.create opts do
# Add static paths
add root_path, changefreq: "daily", priority: 1.0
add new_gemmy_path, changefreq: "monthly"
add new_lockfile_path, changefreq: "monthly"
SitemapGenerator::Sitemap.create opts do
# Add static paths
add root_path, changefreq: "daily", priority: 1.0
add new_gemmy_path, changefreq: "monthly"
add new_lockfile_path, changefreq: "monthly"

# Add dynamic paths for all gemmies
Gemmy.find_each do |gemmy|
add gemmy_path(gemmy), lastmod: gemmy.last_checked_at, changefreq: "weekly", priority: 0.8
# Add dynamic paths for all gemmies
Gemmy.find_each do |gemmy|
add gemmy_path(gemmy), lastmod: gemmy.last_checked_at, changefreq: "weekly", priority: 0.8

rails_releases.each do |rails_release|
add gemmy_rails_release_path(gemmy, rails_release)
rails_releases.each do |rails_release|
add gemmy_rails_release_path(gemmy, rails_release)
end
end
end
end
end
8 changes: 4 additions & 4 deletions spec/controllers/gemmies_controllers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
context "when the gemmy params are valid" do
it "redirects to the new gemmy page" do
post :create, params: { gemmy: { name: "next_rails" } }
expect(response).to redirect_to(gemmy_path(Gemmy.find_by(name: "next_rails")))

expect(response).to redirect_to(gemmy_path(Gemmy.find_by_name("next_rails")))
end

it "creates a record in the database" do
Expand All @@ -18,7 +18,7 @@
context "when the gemmy params are invalid" do
it "renders the new gemmy page" do
post :create, params: { gemmy: { name: "" } }

expect(response).to render_template(:new)
end

Expand All @@ -30,4 +30,4 @@
end
end
end
end
end