Skip to content

Commit

Permalink
Merge branch 'TAN-540-voting-ui-improvements' into TAN-576-show-top-n…
Browse files Browse the repository at this point in the history
…av-on-scroll-up
  • Loading branch information
amanda-anderson committed Dec 6, 2023
2 parents 491503e + b941e2c commit 9624e21
Show file tree
Hide file tree
Showing 58 changed files with 6,318 additions and 4,664 deletions.
2 changes: 1 addition & 1 deletion back/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ gem 'activerecord-import', '~> 1.4'
gem 'json-schema', '~> 4.0'
gem 'activerecord-postgis-adapter', '~> 8.0'

gem 'carrierwave', '~> 3.0.0.beta'
gem 'carrierwave', '~> 3.0.5'
gem 'carrierwave-base64', '~> 2.10'
gem 'fog-aws', '~> 3.19'

Expand Down
8 changes: 4 additions & 4 deletions back/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ GEM
rack-test (>= 0.6.3)
regexp_parser (>= 1.5, < 3.0)
xpath (~> 3.2)
carrierwave (3.0.0.beta)
carrierwave (3.0.5)
activemodel (>= 6.0.0)
activesupport (>= 6.0.0)
addressable (~> 2.6)
Expand Down Expand Up @@ -1098,7 +1098,7 @@ GEM
ruby-saml (1.14.0)
nokogiri (>= 1.10.5)
rexml
ruby-vips (2.1.4)
ruby-vips (2.2.0)
ffi (~> 1.12)
ruby2_keywords (0.0.5)
rubyXL (3.4.25)
Expand Down Expand Up @@ -1158,7 +1158,7 @@ GEM
spring-watcher-listen (2.1.0)
listen (>= 2.7, < 4.0)
spring (>= 4)
ssrf_filter (1.1.1)
ssrf_filter (1.1.2)
stackprof (0.2.25)
swd (2.0.2)
activesupport (>= 3)
Expand Down Expand Up @@ -1248,7 +1248,7 @@ DEPENDENCIES
bunny (>= 2.7.2)
byebug
capybara (~> 3.39)
carrierwave (~> 3.0.0.beta)
carrierwave (~> 3.0.5)
carrierwave-base64 (~> 2.10)
cloudfront-rails (~> 0.4)
combine_pdf
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

module CarrierwaveErrorDetailsTransformation
# @param [ActiveModel::Errors] errors
# @param [Symbol] attribute_name
def transform_carrierwave_error_details(errors, attribute_name)
# @file.errors.details always returns { error: ActiveModel::Error#type }
# But Carrierwave's ActiveModel::Error items sometimes have more specific messages in
# ActiveModel::Error#options[:message] (e.g., max_size_error in this case).
#
# We want to use these specific messages on the FE.
#
# > @file.errors
# => #<ActiveModel::Errors [
# #<ActiveModel::Error attribute=file, type=carrierwave_integrity_error, options={:message=>"max_size_error"}>,
# #<ActiveModel::Error attribute=file, type=blank, options={:unless=>#<Proc:0x0000558d8a77ac00 /cl2_back/app/models/project_file.rb:30>}>
# ]>
#
# Taken from https://github.com/rails/rails/blob/d86b098a8a1/activemodel/lib/active_model/errors.rb#L276
error_details = errors.group_by_attribute.transform_values do |attribute_errors|
attribute_errors.map do |error|
details = error.details
# Here's how these Carrierwave messages are generated https://github.com/carrierwaveuploader/carrierwave/blob/f5b09b844d99245a3b4d0ba01efd4972be4ee5be/lib/carrierwave/uploader/file_size.rb#L36
details[:error] = error.options[:message] || details[:error] if error.attribute == attribute_name
details
end
end
error_details[attribute_name] = error_details[attribute_name]&.uniq { |e| e[:error] }
error_details
end
end
11 changes: 3 additions & 8 deletions back/app/controllers/web_api/v1/files_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class WebApi::V1::FilesController < ApplicationController
include CarrierwaveErrorDetailsTransformation

skip_before_action :authenticate_user

CONSTANTIZER = {
Expand Down Expand Up @@ -78,7 +80,7 @@ def create
params: jsonapi_serializer_params
).serializable_hash, status: :created
else
render json: { errors: transform_errors_details!(@file.errors.details) }, status: :unprocessable_entity
render json: { errors: transform_carrierwave_error_details(@file.errors, :file) }, status: :unprocessable_entity
end
end

Expand Down Expand Up @@ -120,13 +122,6 @@ def set_container
@container = secure_constantize(:container_class).find container_id
end

def transform_errors_details!(error_details)
# carrierwave does not return the error code symbols by default
error_details = error_details.dup
error_details[:file] = error_details[:file]&.uniq { |e| e[:error] }
error_details
end

def secure_constantize(key)
CONSTANTIZER.fetch(params[:container_type])[key]
end
Expand Down
13 changes: 4 additions & 9 deletions back/app/controllers/web_api/v1/images_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class WebApi::V1::ImagesController < ApplicationController
include CarrierwaveErrorDetailsTransformation

CONSTANTIZER = {
'Idea' => {
container_class: Idea,
Expand Down Expand Up @@ -66,7 +68,7 @@ def create
if @image.errors.details[:image].include?({ error: 'processing_error' })
ErrorReporter.report_msg(@image.errors.details.to_s)
end
render json: { errors: transform_errors_details!(@image.errors.details) }, status: :unprocessable_entity
render json: { errors: transform_carrierwave_error_details(@image.errors, :image) }, status: :unprocessable_entity
end
end

Expand All @@ -77,7 +79,7 @@ def update
params: jsonapi_serializer_params
).serializable_hash, status: :ok
else
render json: { errors: transform_errors_details!(@image.errors.details) }, status: :unprocessable_entity
render json: { errors: transform_carrierwave_error_details(@image.errors, :image) }, status: :unprocessable_entity
end
end

Expand Down Expand Up @@ -109,13 +111,6 @@ def set_container
@container = secure_constantize(:container_class).find(container_id)
end

def transform_errors_details!(error_details)
# carrierwave does not return the error code symbols by default
error_details = error_details.dup
error_details[:image] = error_details[:image]&.uniq { |e| e[:error] }
error_details
end

def secure_constantize(key)
CONSTANTIZER.fetch(params[:container_type])[key]
end
Expand Down
2 changes: 1 addition & 1 deletion back/app/uploaders/base_image_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def filename
end

def content_type_allowlist
ALLOWED_TYPES
ALLOWED_TYPES.map { "image/#{_1}" }
end

def extension_allowlist
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module ContentBuilder
class WebApi::V1::LayoutImagesController < ApplicationController
include CarrierwaveErrorDetailsTransformation

def create
@image = LayoutImage.new image_params
authorize @image
Expand All @@ -17,7 +19,7 @@ def create
if @image.errors.details[:image].include?({ error: 'processing_error' })
ErrorReporter.report_msg @image.errors.details.to_s
end
render json: { errors: transform_errors_details!(@image.errors.details) }, status: :unprocessable_entity
render json: { errors: transform_carrierwave_error_details(@image.errors, :image) }, status: :unprocessable_entity
end
end

Expand All @@ -30,12 +32,5 @@ def image_params
def sidefx
@sidefx ||= SideFxLayoutImageService.new
end

def transform_errors_details!(error_details)
# carrierwave does not return the error code symbols by default
error_details = error_details.dup
error_details[:image] = error_details[:image]&.uniq { |e| e[:error] }
error_details
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,12 @@ def self.enabled_by_default
type: 'string',
private: true
}

add_setting 'neighbourhood_custom_field_key', required: false, schema: {
private: true,
type: 'string',
title: 'Neighbourhood custom field key',
description: 'The `key` attribute of the custom field where the neighbourhood should be stored. Leave empty to not store the neighbourhood.'
}
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,21 @@
module IdHoplr
class HoplrOmniauth < OmniauthMethods::Base
def profile_to_user_attrs(auth)
first_name, last_name = auth.info.name.split(' ', 2)
locale = AppConfiguration.instance.settings.dig('core', 'locales').first
settings = AppConfiguration.instance.settings

custom_field_values = {}

neighbourhood = auth.extra.raw_info['neighbourhood']
if (neighbourhood_key = settings.dig('hoplr_login', 'neighbourhood_custom_field_key')) && neighbourhood
custom_field_values[neighbourhood_key] = neighbourhood
end

{
first_name: first_name,
last_name: last_name,
locale: locale,
email: auth.info.email
first_name: auth.info.first_name,
last_name: auth.info.last_name,
locale: settings.dig('core', 'locales').first,
email: auth.info.email,
custom_field_values: custom_field_values
}
end

Expand All @@ -21,7 +28,11 @@ def omniauth_setup(configuration, env)
feature = configuration.settings('hoplr_login')

options = env['omniauth.strategy'].options
options[:scope] = %i[openid email profile]

scope = %i[openid email profile]
scope << :neighbourhood if feature['neighbourhood_custom_field_key'].present?
options[:scope] = scope

# it gets configuration from https://test.hoplr.com/.well-known/openid-configuration
options[:discovery] = true

Expand Down Expand Up @@ -52,7 +63,11 @@ def issuer
end

def updateable_user_attrs
%i[first_name last_name]
%i[first_name last_name custom_field_values]
end

def locked_custom_fields
%i[neighbourhood]
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
'provider' => 'hoplr',
'uid' => '602119',
'info' => {
'name' => 'Alejandro de CitizenLab',
'email' => 'alejandro@citizenlab.co',
'name' => 'Alexander CitizenLab',
'email' => 'alexander@citizenlab.co',
'email_verified' => nil,
'nickname' => nil,
'first_name' => nil,
'last_name' => nil,
'first_name' => 'Alexander',
'last_name' => 'CitizenLab',
'gender' => nil,
'image' => nil,
'image' => 'https://devhoplrstorage.blob.core.windows.net/images/defaultuser.jpg',
'phone' => nil,
'urls' => { 'website' => nil }
},
Expand All @@ -32,18 +32,22 @@
'extra' => {
'raw_info' => {
'sub' => '602119',
'email' => 'alejandro@citizenlab.co',
'email' => 'alexander@citizenlab.co',
'family_name' => 'CitizenLab',
'given_name' => 'Alexander',
'iss' => 'https://test.hoplr.com/',
'aud' => 'ncW9MwpSxxxdh5&Ko3wDJ',
'oi_au_id' => '69c0b378-xxxx-40ea-adc7-61449ae34d60',
'name' => 'Alejandro de CitizenLab',
'role' => 'User',
'azp' => 'ncW9Mwpxxxtdh5&Ko3wDJ',
'nonce' => '9b68944e34406aa6007ee2d4e000083c',
'at_hash' => '6kRGQBCXXXXrDCwMLohXKA',
'oi_tkn_id' => '60cd647a-xxxx-40c0-9743-2d10bc051605',
'exp' => 1_691_493_755,
'iat' => 1_691_492_555
'aud' => 'ncW9xxxxxxxxxx&Ko3wDJ',
'neighbourhood' => '2133',
'oi_au_id' => '2ae1e1e0-0000-4bd5-9317-374e6c82cf23',
'name' => 'Alexander CitizenLab',
'picture' => 'https://devhoplrstorage.blob.core.windows.net/images/defaultuser.jpg',
'locale' => 'en',
'azp' => 'ncW9xxxxxxxxxx&Ko3wDJ',
'nonce' => 'bba2590000000000eeb5e6f1f2c43d93',
'at_hash' => 'NXoxxxxxxxxxxZ8kGx2INg',
'oi_tkn_id' => '5eef5151-0000-4577-82a4-0f9fb3296dd9',
'exp' => 1_701_442_629,
'iat' => 1_701_441_429
}
}
}
Expand All @@ -56,7 +60,8 @@
enabled: true,
environment: 'test',
client_id: 'fakeid',
client_secret: 'fakesecret'
client_secret: 'fakesecret',
neighbourhood_custom_field_key: 'neighbourhood'
}
configuration.save!
host! 'example.org'
Expand All @@ -70,9 +75,13 @@ def expect_user_to_have_attributes(user)
})
expect(user).to have_attributes({
verified: false,
first_name: 'Alejandro',
last_name: 'de CitizenLab',
email: 'alejandro@citizenlab.co'
first_name: 'Alexander',
last_name: 'CitizenLab',
email: 'alexander@citizenlab.co',
locale: 'en',
custom_field_values: {
'neighbourhood' => '2133'
}
})
end

Expand All @@ -87,7 +96,7 @@ def expect_user_to_have_attributes(user)
end

context 'when user already exists' do
let!(:user) { create(:user, email: 'alejandro@citizenlab.co') }
let!(:user) { create(:user, email: 'alexander@citizenlab.co') }

it 'successfully authenticates and updates existing user' do
get '/auth/hoplr'
Expand Down
2 changes: 1 addition & 1 deletion back/engines/free/email_campaigns/config/locales/nl-NL.yml
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ nl:
permissions: 'Machtigingen'
projects: 'Projecten'
proposals: 'Voorstellen'
reactions: 'Reacties'
reactions: 'Likes/dislikes'
voting: 'Stemming'
trigger:
comment_is_deleted: 'Reactie is verwijderd'
Expand Down
4 changes: 2 additions & 2 deletions back/spec/uploaders/base_image_uploader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
RSpec.describe BaseImageUploader do
let(:uploader) { described_class.new }

it 'whitelists exactly [jpg jpeg gif png webp svg]' do
it 'whitelists exactly [image/jpg image/jpeg image/gif image/png image/webp image/svg]' do
expect(uploader.extension_allowlist).to match_array %w[jpg jpeg gif png webp svg]
expect(uploader.content_type_allowlist).to match_array %w[jpg jpeg gif png webp svg]
expect(uploader.content_type_allowlist).to match_array %w[image/jpg image/jpeg image/gif image/png image/webp image/svg]
expect(uploader.extension_denylist).to be_blank
expect(uploader.content_type_denylist).to be_blank
end
Expand Down
Loading

0 comments on commit 9624e21

Please sign in to comment.