From 0235f2f7e66b2855990b04c4c9e0575a5f13fe8f Mon Sep 17 00:00:00 2001 From: franpb14 Date: Tue, 6 Dec 2022 14:52:03 +0100 Subject: [PATCH 1/3] minor refactor in petitions controller, fix tests & tests added --- app/controllers/petitions_controller.rb | 2 +- app/views/petitions/manage.html.erb | 4 +- .../organizations_controller_spec.rb | 11 +++++ spec/controllers/petitions_controller_spec.rb | 49 +++++++++++++++++++ spec/controllers/users_controller_spec.rb | 13 +++++ 5 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 spec/controllers/petitions_controller_spec.rb diff --git a/app/controllers/petitions_controller.rb b/app/controllers/petitions_controller.rb index 4ddb81cd..292c58af 100644 --- a/app/controllers/petitions_controller.rb +++ b/app/controllers/petitions_controller.rb @@ -16,7 +16,7 @@ def update status = params[:status] if petition.update(status: status) - User.find(params[:user_id]).add_to_organization(current_organization) if status == 'accepted' + petition.user.add_to_organization(petition.organization) if status == 'accepted' flash[:notice] = "Application #{status}" else flash[:error] = 'Something went wrong' diff --git a/app/views/petitions/manage.html.erb b/app/views/petitions/manage.html.erb index 71eb4314..2b0500de 100644 --- a/app/views/petitions/manage.html.erb +++ b/app/views/petitions/manage.html.erb @@ -65,8 +65,8 @@ <%= phone_to user.phone %> <% if current_user.manages?(current_organization) && @status == 'pending' %> - <%= link_to 'Accept', petition_path(id: petition.id, user_id: user.id, status: 'accepted'), class: 'btn btn-primary', method: :put %> - <%= link_to 'Decline', petition_path(id: petition.id, user_id: user.id, status: 'declined'), class: 'btn btn-danger', method: :put %> + <%= link_to 'Accept', petition_path(id: petition.id, status: 'accepted'), class: 'btn btn-primary', method: :put %> + <%= link_to 'Decline', petition_path(id: petition.id, status: 'declined'), class: 'btn btn-danger', method: :put %> <% end %> <% end %> diff --git a/spec/controllers/organizations_controller_spec.rb b/spec/controllers/organizations_controller_spec.rb index 66df6030..e09b320d 100644 --- a/spec/controllers/organizations_controller_spec.rb +++ b/spec/controllers/organizations_controller_spec.rb @@ -61,6 +61,17 @@ expect(assigns(:organizations)).to eq([second_organization]) end end + + context 'a user is logged' do + before { login(member.user) } + + it 'populates an array of user organizations' do + get :index + + expect(assigns(:user_organizations)).to include(member.organization) + expect(assigns(:organizations)).to eq([second_organization]) + end + end end describe 'GET #show' do diff --git a/spec/controllers/petitions_controller_spec.rb b/spec/controllers/petitions_controller_spec.rb new file mode 100644 index 00000000..b3612fca --- /dev/null +++ b/spec/controllers/petitions_controller_spec.rb @@ -0,0 +1,49 @@ +RSpec.describe PetitionsController do + let!(:organization) { Fabricate(:organization) } + let(:user) { Fabricate(:user) } + let(:admin) { Fabricate(:member, organization: organization, manager: true) } + + describe 'POST #create' do + before { login(user) } + + it 'creates the petition' do + expect do + post :create, params: { user_id: user.id, organization_id: organization.id } + end.to change(Petition, :count).by(1) + end + end + + describe 'PUT #update' do + before { login(admin.user) } + let(:petition) { Petition.create(user: user, organization: organization, status: 'pending') } + + it 'decline the petition' do + put :update, params: { status: 'declined', id: petition.id } + + petition.reload + expect(petition.status).to eq('declined') + end + + it 'accept the petition and add the user to the org' do + put :update, params: { status: 'accepted', id: petition.id } + + petition.reload + expect(user.members.last.organization.id).to eq(organization.id) + expect(petition.status).to eq('accepted') + end + end + + describe 'GET #manage' do + before do + allow(controller).to receive(:current_organization) { organization } + login(admin.user) + end + let!(:petition) { Petition.create(user: user, organization: organization, status: 'pending') } + + it 'populates a list of users with pending petitions' do + get :manage + + expect(assigns(:users)).to include(user) + end + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 16676d9f..287d09f5 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -249,6 +249,8 @@ end describe "POST #create" do + before { allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return('/edit') } + context "with empty email" do subject do @@ -320,10 +322,21 @@ end end + + context 'with no logged user' do + before { allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return(signup_users_path) } + + it 'creates the user' do + expect { post :create, params: { user: Fabricate.to_params(:user, password: '1234test') } }.to change(User, :count).by(1) + expect(subject).to redirect_to(terms_path) + end + end end end describe "PUT #update" do + before { allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return('/edit') } + context "with valid params" do context "with a logged" do context "normal user" do From 370ce69ebf9233e2a0c05278bbd0bd7f4a1c2c63 Mon Sep 17 00:00:00 2001 From: franpb14 Date: Tue, 6 Dec 2022 15:16:39 +0100 Subject: [PATCH 2/3] hide/show password --- app/assets/stylesheets/application.scss | 13 +++++++++++++ app/views/users/_form.html.erb | 12 +++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index dec301cc..982c4477 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -669,3 +669,16 @@ label[required]::after{ display: inline; margin: 0 !important; } + +.input__password-eye { + display: flex; + align-items: center; + justify-content: right; + + .show-password { + position: absolute; + display: flex; + margin-right: 12px; + cursor: pointer; + } +} diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index 5b9de405..4d119b03 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -12,7 +12,17 @@ <%= f.input :email, readonly: true %> <% end %> - <%= f.input :password, label: t("application.login_form.password"), required: true if short %> + <% if short %> +
+ +
+ + + visibility + +
+
+ <% end %> <%= f.input :phone %> <%= f.input :alt_phone %> From 232761e0e54d74f930fca97578b275e0d72b409c Mon Sep 17 00:00:00 2001 From: franpb14 Date: Tue, 6 Dec 2022 16:40:51 +0100 Subject: [PATCH 3/3] signup user validations and changes in controller related --- app/controllers/users_controller.rb | 10 ++++++---- app/models/user.rb | 8 +++++++- app/views/users/_form.html.erb | 1 + spec/controllers/users_controller_spec.rb | 6 +++--- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 46d4b507..d9013a8b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -33,15 +33,17 @@ def edit def create authorize User + from_signup = params[:from_signup].present? email = user_params[:email] @user = User.find_or_initialize_by(email: email) do |u| u.attributes = user_params end empty_email = @user.email.empty? + @user.from_signup = from_signup @user.setup_and_save_user if @user.persisted? - unless request.referer.include?(signup_users_path) + unless from_signup @user.tune_after_persisted(current_organization) @user.add_tags(current_organization, params[:tag_list] || []) end @@ -50,7 +52,7 @@ def create else @user.email = "" if empty_email - render action: "new" + render action: from_signup ? 'signup' : 'new' end end @@ -108,7 +110,7 @@ def user_params fields_to_permit += %w"admin registration_number registration_date" if admin? fields_to_permit += %w"organization_id superadmin" if superadmin? - fields_to_permit += %w"password" if request.referer.include?(signup_users_path) + fields_to_permit += %w"password" if params[:from_signup].present? params.require(:user).permit *fields_to_permit end @@ -122,7 +124,7 @@ def find_user end def redirect_to_after_create - if request.referer.include?(signup_users_path) + if params[:from_signup].present? sign_in(@user) redirect_to terms_path else diff --git a/app/models/user.rb b/app/models/user.rb index fd1ba040..156743fc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,6 +17,7 @@ class User < ApplicationRecord ) attr_accessor :empty_email + attr_accessor :from_signup has_one_attached :avatar has_many :members, dependent: :destroy @@ -38,6 +39,7 @@ class User < ApplicationRecord validates :username, presence: true validates :email, presence: true, uniqueness: true + validates :password, presence: true, if: :from_signup? # Allows @domain.com for dummy emails but does not allow pure invalid # emails like 'without email' validates_format_of :email, @@ -99,7 +101,7 @@ def setup_and_save_user # temporary valid email with current time milliseconds # this will be updated to user.id@example.com later on self.empty_email = email.strip.empty? - self.email = "user#{DateTime.now.strftime('%Q')}@example.com" if empty_email + self.email = "user#{DateTime.now.strftime('%Q')}@example.com" if empty_email && !from_signup skip_confirmation! # auto-confirm, not sending confirmation email save end @@ -128,4 +130,8 @@ def email_if_real def was_member?(petition) petition.status == 'accepted' && Member.where(organization: petition.organization, user: self).none? end + + def from_signup? + from_signup + end end diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index 4d119b03..08ff149a 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -22,6 +22,7 @@ + <%= hidden_field_tag 'from_signup', 'true' %> <% end %> <%= f.input :phone %> diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 287d09f5..f8a3f7a2 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -249,8 +249,6 @@ end describe "POST #create" do - before { allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return('/edit') } - context "with empty email" do subject do @@ -327,7 +325,9 @@ before { allow_any_instance_of(ActionController::TestRequest).to receive(:referer).and_return(signup_users_path) } it 'creates the user' do - expect { post :create, params: { user: Fabricate.to_params(:user, password: '1234test') } }.to change(User, :count).by(1) + expect do + post :create, params: { user: Fabricate.to_params(:user, password: '1234test'), from_signup: 'true' } + end.to change(User, :count).by(1) expect(subject).to redirect_to(terms_path) end end