Merge pull request #1099 from concord-consortium/misc-project-admin-c…

…leanup

fix: Misc updates to project admin code

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, {

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 {

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
 

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

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)

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

app/views/admin/users/edit.html.erb

@@ -4,6 +4,6 @@
   <%= render :partial => 'form', :locals => { :f => f } %>
 <%- end -%>
 
-<div style="margin-top: 20px;">
+<div class='bottom-links'>
   <%= link_to '‹ Back to Users List', admin_users_path %>
 </div>

app/views/admin/users/new.html.erb

@@ -4,6 +4,6 @@
   <%= render :partial => 'form', :locals => { :f => f } %>
 <%- end -%>
 
-<div style="margin-top: 20px;">
+<div class='bottom-links'>
   <%= link_to '‹ Back to Users List', admin_users_path %>
 </div>

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"} });

lara-typescript/src/projects/components/project-settings-form.tsx

@@ -70,7 +70,7 @@ export const ProjectSettingsForm: React.FC<IProjectSettingsFormProps> = ({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<IProjectSettingsFormProps> = ({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",

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)