From 8d4623c2270043cedb42a97005c521ec9cc5e384 Mon Sep 17 00:00:00 2001 From: Doug Martin Date: Tue, 21 Nov 2023 08:50:23 -0500 Subject: [PATCH] fix: Misc updates to project admin code - Added default_scope for Project to order by title - Updated manual styling of user admin link html container - Removed unneeded attr_accessibles in project_admin model - Changed from passing admin_ids to admin objects to projects api controller - Removed unneeded fallback to empty array of admins in project settings controller --- app/assets/javascripts/lara-typescript.js | 6 ++---- app/assets/stylesheets/style.scss | 3 +++ app/controllers/admin/users_controller.rb | 4 ++-- app/controllers/api/v1/projects_controller.rb | 10 +++++++--- app/models/project.rb | 2 ++ app/models/project_admin.rb | 2 +- app/views/admin/users/edit.html.erb | 2 +- app/views/admin/users/new.html.erb | 2 +- .../components/project-settings-form.spec.tsx | 4 ++-- .../projects/components/project-settings-form.tsx | 4 ++-- spec/controllers/api/v1/projects_controller_spec.rb | 12 ++++++------ 11 files changed, 29 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/lara-typescript.js b/app/assets/javascripts/lara-typescript.js index b895c86a0..18cf493ab 100644 --- a/app/assets/javascripts/lara-typescript.js +++ b/app/assets/javascripts/lara-typescript.js @@ -124689,7 +124689,7 @@ var ProjectSettingsForm = function (_a) { delete data.project.created_at; delete data.project.updated_at; setProject((0, convert_keys_1.snakeToCamelCaseKeys)(data.project)); - setAdmins(data.admins || []); + setAdmins(data.admins); setProjectLoaded(true); setPageTitle("Edit " + data.project.title); return [2 @@ -124763,9 +124763,7 @@ var ProjectSettingsForm = function (_a) { case 0: apiUrl = id ? "/api/v1/projects/" + id : "/api/v1/projects"; projectData = (0, convert_keys_1.camelToSnakeCaseKeys)(project); - projectData.admin_ids = admins.map(function (a) { - return a.id; - }); + projectData.admins = admins; return [4 /*yield*/ , fetch(apiUrl, { diff --git a/app/assets/stylesheets/style.scss b/app/assets/stylesheets/style.scss index cc1ece63c..c06e3046c 100644 --- a/app/assets/stylesheets/style.scss +++ b/app/assets/stylesheets/style.scss @@ -1238,6 +1238,9 @@ body.admin.edit, body.admin.new { padding: 7px; } } + div.bottom-links { + margin-top: 20px; + } } .settings-table { diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 38cfd06b1..58e295688 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -32,7 +32,7 @@ def show # GET /admin/users/new.json def new @user = User.new - @projects = Project.order(:title) + @projects = Project.all authorize! :create, User respond_to do |format| @@ -44,7 +44,7 @@ def new # GET /admin/users/1/edit def edit @user = User.find(params[:id]) - @projects = Project.order(:title) + @projects = Project.all authorize! :update, @user end diff --git a/app/controllers/api/v1/projects_controller.rb b/app/controllers/api/v1/projects_controller.rb index a4cde87ae..7ff8d8e52 100644 --- a/app/controllers/api/v1/projects_controller.rb +++ b/app/controllers/api/v1/projects_controller.rb @@ -4,7 +4,7 @@ class Api::V1::ProjectsController < API::APIController # GET /api/v1/projects def index - @projects = Project.order(:title) + @projects = Project.all authorize! :manage, @projects render json: {projects: @projects} end @@ -37,8 +37,12 @@ def update @project = Project.find(@updated_project_hash[:id]); authorize! :update, @project - # remove any extra admin ids in the update that are not in the original, to ensure we can only remove and not add project admins - @updated_project_hash[:admin_ids] = (@updated_project_hash[:admin_ids] || []).select { |id| @project.admin_ids.include?(id.to_i) } + # extract the ids from the passed admin objects and remove any extra admins in the update that + # are not in the original, to ensure we can only remove and not add project admins + @updated_project_hash[:admin_ids] = (@updated_project_hash[:admins] || []) + .select { |admin| @project.admin_ids.include?(admin[:id].to_i) } + .map { |admin| admin[:id] } + @updated_project_hash.delete(:admins) if @project.update_attributes(@updated_project_hash) render json: {success: true, project: @project, admins: admin_json(@project)}, status: :ok diff --git a/app/models/project.rb b/app/models/project.rb index baee61f50..be1438a26 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -10,6 +10,8 @@ class Project < ActiveRecord::Base has_many :project_admins has_many :admins, through: :project_admins, :source => :user + default_scope order('title') + protected def self.create_default self.create(:title => DefaultName, :logo_lara => '', :url => 'https://concord.org/', :project_key => DefaultKey) diff --git a/app/models/project_admin.rb b/app/models/project_admin.rb index b483a8fe6..125151b55 100644 --- a/app/models/project_admin.rb +++ b/app/models/project_admin.rb @@ -1,5 +1,5 @@ class ProjectAdmin < ActiveRecord::Base - attr_accessible :user, :project, :user_id, :project_id + attr_accessible :user, :project belongs_to :user belongs_to :project diff --git a/app/views/admin/users/edit.html.erb b/app/views/admin/users/edit.html.erb index 2e7a76d77..577d9470c 100644 --- a/app/views/admin/users/edit.html.erb +++ b/app/views/admin/users/edit.html.erb @@ -4,6 +4,6 @@ <%= render :partial => 'form', :locals => { :f => f } %> <%- end -%> -
+
<%= link_to '‹ Back to Users List', admin_users_path %>
diff --git a/app/views/admin/users/new.html.erb b/app/views/admin/users/new.html.erb index fc507acdb..c2588065b 100644 --- a/app/views/admin/users/new.html.erb +++ b/app/views/admin/users/new.html.erb @@ -4,6 +4,6 @@ <%= render :partial => 'form', :locals => { :f => f } %> <%- end -%> -
+
<%= link_to '‹ Back to Users List', admin_users_path %>
\ No newline at end of file diff --git a/lara-typescript/src/projects/components/project-settings-form.spec.tsx b/lara-typescript/src/projects/components/project-settings-form.spec.tsx index ff9c906b5..372dfcf39 100644 --- a/lara-typescript/src/projects/components/project-settings-form.spec.tsx +++ b/lara-typescript/src/projects/components/project-settings-form.spec.tsx @@ -97,8 +97,8 @@ describe("ProjectSettingsForm", () => { const titleInput = container.querySelector("#project-title") as HTMLInputElement; const urlInput = container.querySelector("#project-url") as HTMLInputElement; const saveButton = container.querySelector(".save-button") as HTMLButtonElement; - const updatedProject = {...project, title: "Test Project A", url: "https://concord.org/new-path", admins: []}; - fetch.mockResponse(JSON.stringify({project: updatedProject, success: true})); + const updatedProject = {...project, title: "Test Project A", url: "https://concord.org/new-path"}; + fetch.mockResponse(JSON.stringify({project: updatedProject, admins: [], success: true})); await act(async () => { fireEvent.change(titleInput, { target: { value: "Test Project A"} }); fireEvent.change(urlInput, { target: { value: "https://concord.org/new-path"} }); diff --git a/lara-typescript/src/projects/components/project-settings-form.tsx b/lara-typescript/src/projects/components/project-settings-form.tsx index ccf760714..57560cfac 100644 --- a/lara-typescript/src/projects/components/project-settings-form.tsx +++ b/lara-typescript/src/projects/components/project-settings-form.tsx @@ -70,7 +70,7 @@ export const ProjectSettingsForm: React.FC = ({id}: I delete data.project.created_at; delete data.project.updated_at; setProject(snakeToCamelCaseKeys(data.project)); - setAdmins(data.admins || []); + setAdmins(data.admins); setProjectLoaded(true); setPageTitle(`Edit ${data.project.title}`); }; @@ -124,7 +124,7 @@ export const ProjectSettingsForm: React.FC = ({id}: I const handleSaveProject = async () => { const apiUrl = id ? `/api/v1/projects/${id}` : `/api/v1/projects`; const projectData = camelToSnakeCaseKeys(project); - projectData.admin_ids = admins.map(a => a.id); + projectData.admins = admins; const data = await fetch(apiUrl, { method: "POST", diff --git a/spec/controllers/api/v1/projects_controller_spec.rb b/spec/controllers/api/v1/projects_controller_spec.rb index b54ce58ea..f4d217e7c 100644 --- a/spec/controllers/api/v1/projects_controller_spec.rb +++ b/spec/controllers/api/v1/projects_controller_spec.rb @@ -58,8 +58,8 @@ describe "#update" do it "returns a success message and a JSON string when a project is updated" do prev_updated_at = project1.updated_at - admin_ids = project1.admins.map {|a| a.id} - xhr :post, "update", {project: {id: project1.id, title: "New Project Title", admin_ids: admin_ids}} + admins = project1.admins.map {|a| {id: a.id, email: a.email} } + xhr :post, "update", {project: {id: project1.id, title: "New Project Title", admins: admins}} expect(response.status).to eq(200) expect(response.content_type).to eq("application/json") response_body = JSON.parse(response.body, symbolize_names: true) @@ -71,7 +71,7 @@ it "allows removing project admins" do expect(project1.admins.length).to eq(1) - xhr :post, "update", {project: {id: project1.id, title: "New Project Title", admin_ids: []}} + xhr :post, "update", {project: {id: project1.id, title: "New Project Title", admins: []}} expect(response.status).to eq(200) expect(response.content_type).to eq("application/json") response_body = JSON.parse(response.body, symbolize_names: true) @@ -80,9 +80,9 @@ end it "filters out admins that are not already set" do - admin_ids = project1.admins.map {|a| a.id} - admin_ids.push(user2.id) - xhr :post, "update", {project: {id: project1.id, title: "New Project Title", admin_ids: admin_ids}} + admins = project1.admins.map {|a| {id: a.id, email: a.email} } + admins.push({id: user2.id, email: user2.email}) + xhr :post, "update", {project: {id: project1.id, title: "New Project Title", admins: admins}} expect(response.status).to eq(200) expect(response.content_type).to eq("application/json") response_body = JSON.parse(response.body, symbolize_names: true)