Skip to content

Commit

Permalink
Merge pull request #72 from maglevhq/71-unable-to-upload-or-display-u…
Browse files Browse the repository at this point in the history
…ploaded-images-when-using-uuids-and-postgres

Handle UUIDs as primary keys
  • Loading branch information
did committed Dec 12, 2023
2 parents 9e196a0 + 1443c06 commit a1269c8
Show file tree
Hide file tree
Showing 20 changed files with 141 additions and 36 deletions.
6 changes: 3 additions & 3 deletions app/controllers/maglev/api/assets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def index
end

def show
@asset = resources.find(params[:id])
@asset = resources.find(resource_id)
end

def create
Expand All @@ -21,12 +21,12 @@ def create
end

def update
resources.find(params[:id]).update!(asset_params)
resources.find(resource_id).update!(asset_params)
head :ok
end

def destroy
resources.find(params[:id]).destroy!
resources.find(resource_id).destroy!
head :no_content
end

Expand Down
5 changes: 5 additions & 0 deletions app/controllers/maglev/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,10 @@ class ApplicationController < ::ApplicationController
def use_engine_vite?
true
end

def resource_id
# A standard UUID code contains 32 hex digits along with 4 "-"" symbols
Maglev.uuid_as_primary_key? && params[:id] ? params[:id][0..35] : params[:id]
end
end
end
2 changes: 1 addition & 1 deletion app/controllers/maglev/assets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Maglev
class AssetsController < ApplicationController
def show
@asset = Maglev::Asset.find(params[:id])
@asset = Maglev::Asset.find(resource_id)
send_data @asset.download, filename: @asset.filename, type: @asset.content_type
end
end
Expand Down
8 changes: 6 additions & 2 deletions app/frontend/editor/services/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ export default (api) => ({
create: (attributes) => {
let formData = new FormData()
Object.entries(attributes).forEach(([key, value]) =>
formData.append(`asset[${key}]`, value),
formData.append(`asset[${key}]`, value)
)
return api.post('/assets', formData)
return api.post('/assets', formData, {
headers: {
'Content-Type': 'multipart/form-data'
}
})
},
})
3 changes: 2 additions & 1 deletion db/migrate/20200824085207_create_maglev_sites.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class CreateMaglevSites < ActiveRecord::Migration[6.0]
include Maglev::Migration
def change
create_table :maglev_sites do |t|
create_table :maglev_sites, id: primary_key_type do |t|
t.string :name
t.timestamps
end
Expand Down
3 changes: 2 additions & 1 deletion db/migrate/20200824104648_create_maglev_pages.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class CreateMaglevPages < ActiveRecord::Migration[6.0]
include Maglev::Migration
def change
create_table :maglev_pages do |t|
create_table :maglev_pages, id: primary_key_type do |t|
t.string :title
t.string :path
t.string :seo_title
Expand Down
3 changes: 2 additions & 1 deletion db/migrate/20201206172020_create_maglev_assets.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class CreateMaglevAssets < ActiveRecord::Migration[6.0]
include Maglev::Migration
def change
create_table :maglev_assets do |t|
create_table :maglev_assets, id: primary_key_type do |t|
t.string :filename
t.string :content_type
t.integer :width
Expand Down
5 changes: 3 additions & 2 deletions db/migrate/20210830085101_create_maglev_page_paths.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
class CreateMaglevPagePaths < ActiveRecord::Migration[6.0]
include Maglev::Migration
def change
create_table :maglev_page_paths do |t|
t.references :maglev_page
create_table :maglev_page_paths, id: primary_key_type do |t|
t.references :maglev_page, type: foreign_key_type
t.string :locale, null: false
t.string :value, null: false
end
Expand Down
7 changes: 7 additions & 0 deletions lib/maglev.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,12 @@ def uploader
def services(overrides = {})
::Maglev::AppContainer.new(config.services.merge(overrides)).call
end

def uuid_as_primary_key?
return @uuid_as_primary_key unless @uuid_as_primary_key.nil?

config = Rails.configuration.generators
@uuid_as_primary_key = config.options[config.orm][:primary_key_type] == :uuid
end
end
end
1 change: 1 addition & 0 deletions lib/maglev/engine.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require 'vite_ruby'
require 'maglev/migration'

module Maglev
class Engine < ::Rails::Engine
Expand Down
20 changes: 20 additions & 0 deletions lib/maglev/migration.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# frozen_string_literal: true

module Maglev
module Migration
private

def primary_key_type
primary_key_type_setting || :primary_key
end

def foreign_key_type
primary_key_type_setting || :bigint
end

def primary_key_type_setting
config = Rails.configuration.generators
config.options[config.orm][:primary_key_type]
end
end
end
28 changes: 28 additions & 0 deletions spec/controllers/maglev/api/assets_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe Maglev::Api::AssetsController do
routes { Maglev::Engine.routes }

let(:site) { build(:site) }

before do
allow(controller).to receive(:maglev_site).and_return(site)
end

describe 'GET show' do
describe 'Given we use uuids as primary keys in the DB' do
before do
allow(Maglev).to receive(:uuid_as_primary_key?).and_return(true)
end

it 'serves the file' do
asset = create(:asset)
expect(Maglev::Asset).to receive(:find).with('9565604d-be66-4a23-98da-ed1639804103').and_return(asset)
get :show, params: { id: '9565604d-be66-4a23-98da-ed1639804103-myasset', format: 'application/json' }
expect(response.status).to eq 200
end
end
end
end
22 changes: 22 additions & 0 deletions spec/controllers/maglev/assets_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe Maglev::AssetsController do
routes { Maglev::Engine.routes }

describe 'GET show' do
describe 'Given we use uuids as primary keys in the DB' do
before do
allow(Maglev).to receive(:uuid_as_primary_key?).and_return(true)
end

it 'serves the file' do
asset = create(:asset)
expect(Maglev::Asset).to receive(:find).with('9565604d-be66-4a23-98da-ed1639804103').and_return(asset)
get :show, params: { id: '9565604d-be66-4a23-98da-ed1639804103-myasset' }
expect(response.headers['Content-Type']).to eq 'image/jpeg'
end
end
end
end
2 changes: 1 addition & 1 deletion spec/dummy/.ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.0.1
3.1.4
11 changes: 11 additions & 0 deletions spec/dummy/app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,14 @@
class Account < ApplicationRecord
has_secure_password
end

# == Schema Information
#
# Table name: accounts
#
# id :bigint not null, primary key
# email :string
# password_digest :string
# created_at :datetime not null
# updated_at :datetime not null
#
13 changes: 13 additions & 0 deletions spec/dummy/app/models/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,16 @@ def self.maglev_fetch_sold_out(site:, keyword:, max_items: 10)
end
# rubocop:enable Lint/UnusedMethodArgument
end

# == Schema Information
#
# Table name: products
#
# id :bigint not null, primary key
# name :string
# price :float
# sku :string
# sold_out :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
#
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ if Rails.env.development?
show_indexes: 'true',
simple_indexes: 'false',
model_dir: 'app/models',
root_dir: '',
root_dir: '../../',
include_version: 'false',
require: '',
exclude_tests: 'false',
Expand Down
18 changes: 0 additions & 18 deletions spec/factories/maglev/pages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,3 @@
end
end
end

# == Schema Information
#
# Table name: maglev_pages
#
# id :bigint not null, primary key
# lock_version :integer
# meta_description_translations :jsonb
# og_description_translations :jsonb
# og_image_url_translations :jsonb
# og_title_translations :jsonb
# sections_translations :jsonb
# seo_title_translations :jsonb
# title_translations :jsonb
# visible :boolean default(TRUE)
# created_at :datetime not null
# updated_at :datetime not null
#
13 changes: 13 additions & 0 deletions spec/factories/products.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,16 @@
end
end
end

# == Schema Information
#
# Table name: products
#
# id :bigint not null, primary key
# name :string
# price :float
# sku :string
# sold_out :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
#
5 changes: 0 additions & 5 deletions spec/requests/maglev/api/assets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@
delete '/maglev/api/assets/made-up-id'
expect(response).to have_http_status(:not_found)
end

it 'fails when updating missing assets' do
put '/maglev/api/assets/made-up-id'
expect(response).to have_http_status(:not_found)
end
end

describe 'uploads' do
Expand Down

0 comments on commit a1269c8

Please sign in to comment.