From 1ab87030081b54d0725ae2f65fbb5498d2717959 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Fri, 23 Sep 2016 16:41:08 +0800
Subject: [PATCH 01/13] move explore maps methods into their own controller

---
 app/controllers/explore_controller.rb | 114 +++++++++++++++
 app/controllers/maps_controller.rb    | 201 ++++++--------------------
 app/policies/explore_policy.rb        |  25 ++++
 3 files changed, 183 insertions(+), 157 deletions(-)
 create mode 100644 app/controllers/explore_controller.rb
 create mode 100644 app/policies/explore_policy.rb

diff --git a/app/controllers/explore_controller.rb b/app/controllers/explore_controller.rb
new file mode 100644
index 00000000..c21454d1
--- /dev/null
+++ b/app/controllers/explore_controller.rb
@@ -0,0 +1,114 @@
+class ExploreController < ApplicationController
+  before_action :authorize_explore
+  after_action :verify_authorized
+  after_action :verify_policy_scoped
+
+  respond_to :html, :json, :csv
+
+  # TODO remove?
+  #autocomplete :map, :name, full: true, extra_data: [:user_id]
+
+  # GET /explore/active
+  def activemaps
+    page = params[:page].present? ? params[:page] : 1
+    @maps = policy_scope(Map).order('updated_at DESC')
+                             .page(page).per(20)
+
+    respond_to do |format|
+      format.html do
+        # root url => main/home. main/home renders maps/activemaps view.
+        redirect_to(root_url) && return if authenticated?
+        respond_with(@maps, @user)
+      end
+      format.json { render json: @maps }
+    end
+  end
+
+  # GET /explore/featured
+  def featuredmaps
+    page = params[:page].present? ? params[:page] : 1
+    @maps = policy_scope(
+      Map.where('maps.featured = ? AND maps.permission != ?',
+                true, 'private')
+    ).order('updated_at DESC').page(page).per(20)
+
+    respond_to do |format|
+      format.html { respond_with(@maps, @user) }
+      format.json { render json: @maps }
+    end
+  end
+
+  # GET /explore/mine
+  def mymaps
+    unless authenticated?
+      skip_policy_scope
+      return redirect_to explore_active_path
+    end
+
+    page = params[:page].present? ? params[:page] : 1
+    @maps = policy_scope(
+      Map.where('maps.user_id = ?', current_user.id)
+    ).order('updated_at DESC').page(page).per(20)
+
+    respond_to do |format|
+      format.html { respond_with(@maps, @user) }
+      format.json { render json: @maps }
+    end
+  end
+
+  # GET /explore/shared
+  def sharedmaps
+    unless authenticated?
+      skip_policy_scope
+      return redirect_to explore_active_path
+    end
+
+    page = params[:page].present? ? params[:page] : 1
+    @maps = policy_scope(
+      Map.where('maps.id IN (?)', current_user.shared_maps.map(&:id))
+    ).order('updated_at DESC').page(page).per(20)
+
+    respond_to do |format|
+      format.html { respond_with(@maps, @user) }
+      format.json { render json: @maps }
+    end
+  end
+
+  # GET /explore/starred
+  def starredmaps
+    unless authenticated?
+      skip_policy_scope
+      return redirect_to explore_active_path
+    end
+
+    page = params[:page].present? ? params[:page] : 1
+    stars = current_user.stars.map(&:map_id)
+    @maps = policy_scope(
+      Map.where('maps.id IN (?)', stars)
+    ).order('updated_at DESC').page(page).per(20)
+
+    respond_to do |format|
+      format.html { respond_with(@maps, @user) }
+      format.json { render json: @maps }
+    end
+  end
+
+  # GET /explore/mapper/:id
+  def usermaps
+    page = params[:page].present? ? params[:page] : 1
+    @user = User.find(params[:id])
+    @maps = policy_scope(Map.where(user: @user))
+            .order('updated_at DESC').page(page).per(20)
+
+    respond_to do |format|
+      format.html { respond_with(@maps, @user) }
+      format.json { render json: @maps }
+    end
+  end
+
+  private
+
+  def authorize_explore
+    authorize :Explore
+  end
+end
diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb
index 4a091e17..0d308f1d 100644
--- a/app/controllers/maps_controller.rb
+++ b/app/controllers/maps_controller.rb
@@ -1,111 +1,12 @@
 # frozen_string_literal: true
 class MapsController < ApplicationController
   before_action :require_user, only: [:create, :update, :access, :star, :unstar, :screenshot, :events, :destroy]
-  after_action :verify_authorized, except: [:activemaps, :featuredmaps, :mymaps, :sharedmaps, :starredmaps, :usermaps]
-  after_action :verify_policy_scoped, only: [:activemaps, :featuredmaps, :mymaps, :sharedmaps, :starredmaps, :usermaps]
+  after_action :verify_authorized
 
   respond_to :html, :json, :csv
 
   autocomplete :map, :name, full: true, extra_data: [:user_id]
 
-  # GET /explore/active
-  def activemaps
-    page = params[:page].present? ? params[:page] : 1
-    @maps = policy_scope(Map).order('updated_at DESC')
-                             .page(page).per(20)
-
-    respond_to do |format|
-      format.html do
-        # root url => main/home. main/home renders maps/activemaps view.
-        redirect_to(root_url) && return if authenticated?
-        respond_with(@maps, @user)
-      end
-      format.json { render json: @maps.to_json }
-    end
-  end
-
-  # GET /explore/featured
-  def featuredmaps
-    page = params[:page].present? ? params[:page] : 1
-    @maps = policy_scope(
-      Map.where('maps.featured = ? AND maps.permission != ?',
-                true, 'private')
-    ).order('updated_at DESC').page(page).per(20)
-
-    respond_to do |format|
-      format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps.to_json }
-    end
-  end
-
-  # GET /explore/mine
-  def mymaps
-    unless authenticated?
-      skip_policy_scope
-      return redirect_to explore_active_path
-    end
-
-    page = params[:page].present? ? params[:page] : 1
-    @maps = policy_scope(
-      Map.where('maps.user_id = ?', current_user.id)
-    ).order('updated_at DESC').page(page).per(20)
-
-    respond_to do |format|
-      format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps.to_json }
-    end
-  end
-
-  # GET /explore/shared
-  def sharedmaps
-    unless authenticated?
-      skip_policy_scope
-      return redirect_to explore_active_path
-    end
-
-    page = params[:page].present? ? params[:page] : 1
-    @maps = policy_scope(
-      Map.where('maps.id IN (?)', current_user.shared_maps.map(&:id))
-    ).order('updated_at DESC').page(page).per(20)
-
-    respond_to do |format|
-      format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps.to_json }
-    end
-  end
-
-  # GET /explore/starred
-  def starredmaps
-    unless authenticated?
-      skip_policy_scope
-      return redirect_to explore_active_path
-    end
-
-    page = params[:page].present? ? params[:page] : 1
-    stars = current_user.stars.map(&:map_id)
-    @maps = policy_scope(
-      Map.where('maps.id IN (?)', stars)
-    ).order('updated_at DESC').page(page).per(20)
-
-    respond_to do |format|
-      format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps.to_json }
-    end
-  end
-
-  # GET /explore/mapper/:id
-  def usermaps
-    page = params[:page].present? ? params[:page] : 1
-    @user = User.find(params[:id])
-    @maps = policy_scope(Map.where(user: @user))
-            .order('updated_at DESC').page(page).per(20)
-
-    respond_to do |format|
-      format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps.to_json }
-    end
-  end
-
   # GET maps/new
   def new
     @map = Map.new(name: 'Untitled Map', permission: 'public', arranged: true)
@@ -182,78 +83,31 @@ class MapsController < ApplicationController
     @map = Map.find(params[:id])
     authorize @map
 
-    @allmappers = @map.contributors
-    @allcollaborators = @map.editors
-    @alltopics = @map.topics.to_a.delete_if { |t| !policy(t).show? }
-    @allsynapses = @map.synapses.to_a.delete_if { |s| !policy(s).show? }
-    @allmappings = @map.mappings.to_a.delete_if { |m| !policy(m).show? }
-
-    @json = {}
-    @json['map'] = @map
-    @json['topics'] = @alltopics
-    @json['synapses'] = @allsynapses
-    @json['mappings'] = @allmappings
-    @json['mappers'] = @allmappers
-    @json['collaborators'] = @allcollaborators
-    @json['messages'] = @map.messages.sort_by(&:created_at)
-    @json['stars'] = @map.stars
-
     respond_to do |format|
-      format.json { render json: @json }
+      format.json { render json: @map.contains(current_user) }
     end
   end
 
   # POST maps
   def create
     @user = current_user
-    @map = Map.new
-    @map.name = params[:name]
-    @map.desc = params[:desc]
-    @map.permission = params[:permission]
+    @map = Map.new(create_map_params)
     @map.user = @user
     @map.arranged = false
 
-    if params[:topicsToMap]
-      @all = params[:topicsToMap]
-      @all = @all.split(',')
-      @all.each do |topic|
-        topic = topic.split('/')
-        mapping = Mapping.new
-        mapping.map = @map
-        mapping.user = @user
-        mapping.mappable = Topic.find(topic[0])
-        mapping.xloc = topic[1]
-        mapping.yloc = topic[2]
-        authorize mapping, :create?
-        mapping.save
-      end
-
-      if params[:synapsesToMap]
-        @synAll = params[:synapsesToMap]
-        @synAll = @synAll.split(',')
-        @synAll.each do |synapse_id|
-          mapping = Mapping.new
-          mapping.map = @map
-          mapping.user = @user
-          mapping.mappable = Synapse.find(synapse_id)
-          authorize mapping, :create?
-          mapping.save
-        end
-      end
-
+    if params[:topicsToMap].present?
+      create_topics!
+      create_synapses! if params[:synapsesToMap].present?
       @map.arranged = true
     end
 
     authorize @map
 
+    respond_to do |format|
     if @map.save
-      respond_to do |format|
-        format.json { render json: @map }
-      end
+      format.json { render json: @map }
     else
-      respond_to do |format|
-        format.json { render json: 'invalid params' }
-      end
+      format.json { render json: 'invalid params' }
     end
   end
 
@@ -263,7 +117,7 @@ class MapsController < ApplicationController
     authorize @map
 
     respond_to do |format|
-      if @map.update_attributes(map_params)
+      if @map.update_attributes(update_map_params)
         format.json { head :no_content }
       else
         format.json { render json: @map.errors, status: :unprocessable_entity }
@@ -366,7 +220,40 @@ class MapsController < ApplicationController
   private
 
   # Never trust parameters from the scary internet, only allow the white list through.
-  def map_params
+  def create_map_params
+    params.require(:map).permit(:name, :desc, :permission)
+  end
+
+  def update_map_params
     params.require(:map).permit(:id, :name, :arranged, :desc, :permission)
   end
+
+	def create_topics!
+		topics = params[:topicsToMap]
+		topics = topics.split(',')
+		topics.each do |topic|
+			topic = topic.split('/')
+			mapping = Mapping.new
+			mapping.map = @map
+			mapping.user = @user
+			mapping.mappable = Topic.find(topic[0])
+			mapping.xloc = topic[1]
+			mapping.yloc = topic[2]
+			authorize mapping, :create?
+			mapping.save
+		end
+	end
+
+	def create_synapses!
+		@synAll = params[:synapsesToMap]
+		@synAll = @synAll.split(',')
+		@synAll.each do |synapse_id|
+			mapping = Mapping.new
+			mapping.map = @map
+			mapping.user = @user
+			mapping.mappable = Synapse.find(synapse_id)
+			authorize mapping, :create?
+			mapping.save
+		end
+	end
 end
diff --git a/app/policies/explore_policy.rb b/app/policies/explore_policy.rb
new file mode 100644
index 00000000..6cbdab15
--- /dev/null
+++ b/app/policies/explore_policy.rb
@@ -0,0 +1,25 @@
+class ExplorePolicy < ApplicationPolicy
+  def activemaps?
+    true
+  end
+
+  def featuredmaps?
+    true
+  end
+
+  def mymaps?
+    true
+  end
+
+  def sharedmaps?
+    true
+  end
+
+  def starredmaps?
+    true
+  end
+
+  def usermaps?
+    true
+  end
+end

From 40bd9ed95a30996c6785b33688eda02a6ef24d60 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Fri, 23 Sep 2016 16:41:15 +0800
Subject: [PATCH 02/13] refactor maps controller a bit

---
 app/models/map.rb | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/app/models/map.rb b/app/models/map.rb
index 9c30479f..7de744a2 100644
--- a/app/models/map.rb
+++ b/app/models/map.rb
@@ -108,4 +108,23 @@ class Map < ApplicationRecord
     self.screenshot = data
     save
   end
+
+  # user param helps determine what records are visible
+  def contains(user)
+    allmappers = contributors
+    allcollaborators = editors
+    alltopics = Pundit.policy_scope(user, topics).to_a
+    allsynapses = Pundit.policy_scope(user, synapses).to_a
+    allmappings = Pundit.policy_scope(user, mappings).to_a
+
+    json = {}
+    json['map'] = self
+    json['topics'] = alltopics
+    json['synapses'] = allsynapses
+    json['mappings'] = allmappings
+    json['mappers'] = allmappers
+    json['collaborators'] = allcollaborators
+    json['messages'] = messages.sort_by(&:created_at)
+    json['stars'] = stars
+  end
 end

From 7275beb163d585ed733df25d2a41079fbfba40c4 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Fri, 23 Sep 2016 16:48:43 +0800
Subject: [PATCH 03/13] put CRUD at top of maps controller, and alphabetize
 other actions below

---
 app/controllers/maps_controller.rb | 243 +++++++++++++----------------
 1 file changed, 111 insertions(+), 132 deletions(-)

diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb
index 0d308f1d..ff7a1686 100644
--- a/app/controllers/maps_controller.rb
+++ b/app/controllers/maps_controller.rb
@@ -1,12 +1,33 @@
 # frozen_string_literal: true
 class MapsController < ApplicationController
-  before_action :require_user, only: [:create, :update, :access, :star, :unstar, :screenshot, :events, :destroy]
+  before_action :require_user, only: [:create, :update, :destroy, :access, :events, :screenshot, :star, :unstar]
+  before_action :set_map, only: [:show, :update, :destroy, :access, :contains, :events, :export, :screenshot, :star, :unstar]
   after_action :verify_authorized
 
   respond_to :html, :json, :csv
 
   autocomplete :map, :name, full: true, extra_data: [:user_id]
 
+  # GET maps/:id
+  def show
+    respond_to do |format|
+      format.html do
+        @allmappers = @map.contributors
+        @allcollaborators = @map.editors
+        @alltopics = policy_scope(@map.topics)
+        @allsynapses = policy_scope(@map.synapses)
+        @allmappings = policy_scope(@map.mappings)
+        @allmessages = @map.messages.sort_by(&:created_at)
+        @allstars = @map.stars
+
+        respond_with(@allmappers, @allcollaborators, @allmappings, @allsynapses, @alltopics, @allmessages, @allstars, @map)
+      end
+      format.json { render json: @map }
+      format.csv { redirect_to action: :export, format: :csv }
+      format.xls { redirect_to action: :export, format: :xls }
+    end
+  end
+
   # GET maps/new
   def new
     @map = Map.new(name: 'Untitled Map', permission: 'public', arranged: true)
@@ -21,73 +42,6 @@ class MapsController < ApplicationController
     end
   end
 
-  # GET maps/:id
-  def show
-    @map = Map.find(params[:id])
-    authorize @map
-
-    respond_to do |format|
-      format.html do
-        @allmappers = @map.contributors
-        @allcollaborators = @map.editors
-        @alltopics = @map.topics.to_a.delete_if { |t| !policy(t).show? }
-        @allsynapses = @map.synapses.to_a.delete_if { |s| !policy(s).show? }
-        @allmappings = @map.mappings.to_a.delete_if { |m| !policy(m).show? }
-        @allmessages = @map.messages.sort_by(&:created_at)
-        @allstars = @map.stars
-
-        respond_with(@allmappers, @allcollaborators, @allmappings, @allsynapses, @alltopics, @allmessages, @allstars, @map)
-      end
-      format.json { render json: @map }
-      format.csv { redirect_to action: :export, format: :csv }
-      format.xls { redirect_to action: :export, format: :xls }
-    end
-  end
-
-  # GET maps/:id/export
-  def export
-    map = Map.find(params[:id])
-    authorize map
-    exporter = MapExportService.new(current_user, map)
-    respond_to do |format|
-      format.json { render json: exporter.json }
-      format.csv { send_data exporter.csv }
-      format.xls { @spreadsheet = exporter.xls }
-    end
-  end
-
-  # POST maps/:id/events/:event
-  def events
-    map = Map.find(params[:id])
-    authorize map
-
-    valid_event = false
-    if params[:event] == 'conversation'
-      Events::ConversationStartedOnMap.publish!(map, current_user)
-      valid_event = true
-    elsif params[:event] == 'user_presence'
-      Events::UserPresentOnMap.publish!(map, current_user)
-      valid_event = true
-    end
-
-    respond_to do |format|
-      format.json do
-        head :ok if valid_event
-        head :bad_request unless valid_event
-      end
-    end
-  end
-
-  # GET maps/:id/contains
-  def contains
-    @map = Map.find(params[:id])
-    authorize @map
-
-    respond_to do |format|
-      format.json { render json: @map.contains(current_user) }
-    end
-  end
-
   # POST maps
   def create
     @user = current_user
@@ -104,18 +58,16 @@ class MapsController < ApplicationController
     authorize @map
 
     respond_to do |format|
-    if @map.save
-      format.json { render json: @map }
-    else
-      format.json { render json: 'invalid params' }
+      if @map.save
+        format.json { render json: @map }
+      else
+        format.json { render json: 'invalid params' }
+      end
     end
   end
 
   # PUT maps/:id
   def update
-    @map = Map.find(params[:id])
-    authorize @map
-
     respond_to do |format|
       if @map.update_attributes(update_map_params)
         format.json { head :no_content }
@@ -125,10 +77,19 @@ class MapsController < ApplicationController
     end
   end
 
+  # DELETE maps/:id
+  def destroy
+    @map.delete
+
+    respond_to do |format|
+      format.json do
+        head :no_content
+      end
+    end
+  end
+
   # POST maps/:id/access
   def access
-    @map = Map.find(params[:id])
-    authorize @map
     userIds = params[:access] || []
     added = userIds.select do |uid|
       user = User.find(uid)
@@ -155,39 +116,44 @@ class MapsController < ApplicationController
     end
   end
 
-  # POST maps/:id/star
-  def star
-    @map = Map.find(params[:id])
-    authorize @map
-    star = Star.find_by_map_id_and_user_id(@map.id, current_user.id)
-    star = Star.create(map_id: @map.id, user_id: current_user.id) unless star
-
+  # GET maps/:id/contains
+  def contains
     respond_to do |format|
-      format.json do
-        render json: { message: 'Successfully starred map' }
-      end
+      format.json { render json: @map.contains(current_user) }
     end
   end
 
-  # POST maps/:id/unstar
-  def unstar
-    @map = Map.find(params[:id])
-    authorize @map
-    star = Star.find_by_map_id_and_user_id(@map.id, current_user.id)
-    star&.delete
+  # GET maps/:id/export
+  def export
+    exporter = MapExportService.new(current_user, @map)
+    respond_to do |format|
+      format.json { render json: exporter.json }
+      format.csv { send_data exporter.csv }
+      format.xls { @spreadsheet = exporter.xls }
+    end
+  end
+
+  # POST maps/:id/events/:event
+  def events
+    valid_event = false
+    if params[:event] == 'conversation'
+      Events::ConversationStartedOnMap.publish!(@map, current_user)
+      valid_event = true
+    elsif params[:event] == 'user_presence'
+      Events::UserPresentOnMap.publish!(@map, current_user)
+      valid_event = true
+    end
 
     respond_to do |format|
       format.json do
-        render json: { message: 'Successfully unstarred map' }
+        head :bad_request unless valid_event
+        head :ok
       end
     end
   end
 
   # POST maps/:id/upload_screenshot
   def screenshot
-    @map = Map.find(params[:id])
-    authorize @map
-
     png = Base64.decode64(params[:encoded_image]['data:image/png;base64,'.length..-1])
     StringIO.open(png) do |data|
       data.class.class_eval { attr_accessor :original_filename, :content_type }
@@ -203,23 +169,36 @@ class MapsController < ApplicationController
     end
   end
 
-  # DELETE maps/:id
-  def destroy
-    @map = Map.find(params[:id])
-    authorize @map
-
-    @map.delete
+  # POST maps/:id/star
+  def star
+    star = Star.find_or_create_by(map_id: @map.id, user_id: current_user.id)
 
     respond_to do |format|
       format.json do
-        head :no_content
+        render json: { message: 'Successfully starred map' }
+      end
+    end
+  end
+
+  # POST maps/:id/unstar
+  def unstar
+    star = Star.find_by(map_id: @map.id, user_id: current_user.id)
+    star&.delete
+
+    respond_to do |format|
+      format.json do
+        render json: { message: 'Successfully unstarred map' }
       end
     end
   end
 
   private
 
-  # Never trust parameters from the scary internet, only allow the white list through.
+  def set_map
+    @map = Map.find(params[:id])
+    authorize @map
+  end
+
   def create_map_params
     params.require(:map).permit(:name, :desc, :permission)
   end
@@ -228,32 +207,32 @@ class MapsController < ApplicationController
     params.require(:map).permit(:id, :name, :arranged, :desc, :permission)
   end
 
-	def create_topics!
-		topics = params[:topicsToMap]
-		topics = topics.split(',')
-		topics.each do |topic|
-			topic = topic.split('/')
-			mapping = Mapping.new
-			mapping.map = @map
-			mapping.user = @user
-			mapping.mappable = Topic.find(topic[0])
-			mapping.xloc = topic[1]
-			mapping.yloc = topic[2]
-			authorize mapping, :create?
-			mapping.save
-		end
-	end
+  def create_topics!
+    topics = params[:topicsToMap]
+    topics = topics.split(',')
+    topics.each do |topic|
+      topic = topic.split('/')
+      mapping = Mapping.new
+      mapping.map = @map
+      mapping.user = @user
+      mapping.mappable = Topic.find(topic[0])
+      mapping.xloc = topic[1]
+      mapping.yloc = topic[2]
+      authorize mapping, :create?
+      mapping.save
+    end
+  end
 
-	def create_synapses!
-		@synAll = params[:synapsesToMap]
-		@synAll = @synAll.split(',')
-		@synAll.each do |synapse_id|
-			mapping = Mapping.new
-			mapping.map = @map
-			mapping.user = @user
-			mapping.mappable = Synapse.find(synapse_id)
-			authorize mapping, :create?
-			mapping.save
-		end
-	end
+  def create_synapses!
+    @synAll = params[:synapsesToMap]
+    @synAll = @synAll.split(',')
+    @synAll.each do |synapse_id|
+      mapping = Mapping.new
+      mapping.map = @map
+      mapping.user = @user
+      mapping.mappable = Synapse.find(synapse_id)
+      authorize mapping, :create?
+      mapping.save
+    end
+  end
 end

From 686d80e27412bbe9a088a300ed86b57f91810467 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Fri, 23 Sep 2016 17:02:52 +0800
Subject: [PATCH 04/13] move more logic into map model

---
 app/controllers/maps_controller.rb | 31 ++++++---------------------
 app/models/map.rb                  | 34 ++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb
index ff7a1686..1eba68e9 100644
--- a/app/controllers/maps_controller.rb
+++ b/app/controllers/maps_controller.rb
@@ -90,24 +90,13 @@ class MapsController < ApplicationController
 
   # POST maps/:id/access
   def access
-    userIds = params[:access] || []
-    added = userIds.select do |uid|
-      user = User.find(uid)
-      if user.nil? || (current_user && user == current_user)
-        false
-      else
-        !@map.collaborators.include?(user)
-      end
-    end
-    removed = @map.collaborators.select { |user| !userIds.include?(user.id.to_s) }.map(&:id)
-    added.each do |uid|
-      UserMap.create(user_id: uid.to_i, map_id: @map.id)
-      user = User.find(uid.to_i)
-      MapMailer.invite_to_edit_email(@map, current_user, user).deliver_later
-    end
-    removed.each do |uid|
-      @map.user_maps.select { |um| um.user_id == uid }.each(&:destroy)
+    user_ids = params[:access] || []
+
+    added = @map.add_new_collaborators(user_ids)
+    added.each do |user_id|
+      MapMailer.invite_to_edit_email(@map, current_user, User.find(user_id)).deliver_later
     end
+    @map.remove_old_collaborators(user_ids)
 
     respond_to do |format|
       format.json do
@@ -154,13 +143,7 @@ class MapsController < ApplicationController
 
   # POST maps/:id/upload_screenshot
   def screenshot
-    png = Base64.decode64(params[:encoded_image]['data:image/png;base64,'.length..-1])
-    StringIO.open(png) do |data|
-      data.class.class_eval { attr_accessor :original_filename, :content_type }
-      data.original_filename = 'map-' + @map.id.to_s + '-screenshot.png'
-      data.content_type = 'image/png'
-      @map.screenshot = data
-    end
+    @map.base64_screenshot(params[:encoded_image])
 
     if @map.save
       render json: { message: 'Successfully uploaded the map screenshot.' }
diff --git a/app/models/map.rb b/app/models/map.rb
index 7de744a2..ab3f3eb1 100644
--- a/app/models/map.rb
+++ b/app/models/map.rb
@@ -127,4 +127,38 @@ class Map < ApplicationRecord
     json['messages'] = messages.sort_by(&:created_at)
     json['stars'] = stars
   end
+
+  def add_new_collaborators(user_ids)
+    added = []
+    users = User.where(id: user_ids)
+    users.each do |user|
+      if user && user != current_user && !collaborators.include?(user)
+        UserMap.create(user_id: uid.to_i, map_id: id)
+        user = User.find(uid.to_i)
+        added << user.id
+      end
+    end
+    added
+  end
+
+  def remove_old_collaborators(user_ids)
+    removed = []
+    collaborators.map(&:id).each do |user_id|
+      if !user_ids.include?(user_id)
+        user_maps.select { |um| um.user_id == user_id }.each(&:destroy)
+        removed << user_id
+      end
+    end
+    removed
+  end
+
+  def base64_screenshot(encoded_image)
+    png = Base64.decode64(encoded_image['data:image/png;base64,'.length..-1])
+    StringIO.open(png) do |data|
+      data.class.class_eval { attr_accessor :original_filename, :content_type }
+      data.original_filename = 'map-' + @map.id.to_s + '-screenshot.png'
+      data.content_type = 'image/png'
+      @map.screenshot = data
+    end
+  end
 end

From 5e180ac10e5d0348c9b5b4a2f30d865df2331ec2 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Fri, 23 Sep 2016 17:07:06 +0800
Subject: [PATCH 05/13] set up explore controller routes and rename methods

---
 app/controllers/explore_controller.rb | 12 ++++++------
 config/routes.rb                      | 14 ++++++++------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/app/controllers/explore_controller.rb b/app/controllers/explore_controller.rb
index c21454d1..023a6866 100644
--- a/app/controllers/explore_controller.rb
+++ b/app/controllers/explore_controller.rb
@@ -9,7 +9,7 @@ class ExploreController < ApplicationController
   #autocomplete :map, :name, full: true, extra_data: [:user_id]
 
   # GET /explore/active
-  def activemaps
+  def active
     page = params[:page].present? ? params[:page] : 1
     @maps = policy_scope(Map).order('updated_at DESC')
                              .page(page).per(20)
@@ -25,7 +25,7 @@ class ExploreController < ApplicationController
   end
 
   # GET /explore/featured
-  def featuredmaps
+  def featured
     page = params[:page].present? ? params[:page] : 1
     @maps = policy_scope(
       Map.where('maps.featured = ? AND maps.permission != ?',
@@ -39,7 +39,7 @@ class ExploreController < ApplicationController
   end
 
   # GET /explore/mine
-  def mymaps
+  def mine
     unless authenticated?
       skip_policy_scope
       return redirect_to explore_active_path
@@ -57,7 +57,7 @@ class ExploreController < ApplicationController
   end
 
   # GET /explore/shared
-  def sharedmaps
+  def shared
     unless authenticated?
       skip_policy_scope
       return redirect_to explore_active_path
@@ -75,7 +75,7 @@ class ExploreController < ApplicationController
   end
 
   # GET /explore/starred
-  def starredmaps
+  def starred
     unless authenticated?
       skip_policy_scope
       return redirect_to explore_active_path
@@ -94,7 +94,7 @@ class ExploreController < ApplicationController
   end
 
   # GET /explore/mapper/:id
-  def usermaps
+  def mapper
     page = params[:page].present? ? params[:page] : 1
     @user = User.find(params[:id])
     @maps = policy_scope(Map.where(user: @user))
diff --git a/config/routes.rb b/config/routes.rb
index fe48b6ba..f9a1be8f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -57,12 +57,14 @@ Metamaps::Application.routes.draw do
   post 'maps/:id/star', to: 'maps#star', defaults: { format: :json }
   post 'maps/:id/unstar', to: 'maps#unstar', defaults: { format: :json }
 
-  get 'explore/active', to: 'maps#activemaps'
-  get 'explore/featured', to: 'maps#featuredmaps'
-  get 'explore/mine', to: 'maps#mymaps'
-  get 'explore/shared', to: 'maps#sharedmaps'
-  get 'explore/starred', to: 'maps#starredmaps'
-  get 'explore/mapper/:id', to: 'maps#usermaps'
+  namespace :explore do
+    get 'active'
+    get 'featured'
+    get 'mine'
+    get 'shared'
+    get 'starred'
+    get 'mapper/:id', action: 'mapper'
+  end
 
   devise_for :users, skip: :sessions, controllers: {
     registrations: 'users/registrations',

From b722d2d3b07e4a74901b0fd98e723ed55490a047 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Fri, 23 Sep 2016 18:20:06 +0800
Subject: [PATCH 06/13] fix map controller create spec

---
 app/controllers/maps_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb
index 1eba68e9..a74a35c2 100644
--- a/app/controllers/maps_controller.rb
+++ b/app/controllers/maps_controller.rb
@@ -183,7 +183,7 @@ class MapsController < ApplicationController
   end
 
   def create_map_params
-    params.require(:map).permit(:name, :desc, :permission)
+    params.permit(:name, :desc, :permission)
   end
 
   def update_map_params

From 3f9077b3805e77735c9673ea5c81ebb7b47e380f Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sat, 24 Sep 2016 12:35:06 +0800
Subject: [PATCH 07/13] clean up

---
 app/policies/map_policy.rb | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/app/policies/map_policy.rb b/app/policies/map_policy.rb
index 4cb2db38..84d24ca4 100644
--- a/app/policies/map_policy.rb
+++ b/app/policies/map_policy.rb
@@ -41,10 +41,6 @@ class MapPolicy < ApplicationPolicy
     user.present? && record.user == user
   end
 
-  def activemaps?
-    user.blank? # redirect to root url if authenticated for some reason
-  end
-
   def contains?
     show?
   end
@@ -57,14 +53,6 @@ class MapPolicy < ApplicationPolicy
     show?
   end
 
-  def featuredmaps?
-    true
-  end
-
-  def mymaps?
-    user.present?
-  end
-
   def star?
     unstar?
   end
@@ -76,8 +64,4 @@ class MapPolicy < ApplicationPolicy
   def screenshot?
     update?
   end
-
-  def usermaps?
-    true
-  end
 end

From c76de5b1d5cb18add72499ff41d72ae74bbaf4e7 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sat, 24 Sep 2016 12:54:14 +0800
Subject: [PATCH 08/13] refactor map model a bit and fix bugs

---
 app/models/map.rb | 52 ++++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/app/models/map.rb b/app/models/map.rb
index ab3f3eb1..0cab398d 100644
--- a/app/models/map.rb
+++ b/app/models/map.rb
@@ -111,45 +111,37 @@ class Map < ApplicationRecord
 
   # user param helps determine what records are visible
   def contains(user)
-    allmappers = contributors
-    allcollaborators = editors
-    alltopics = Pundit.policy_scope(user, topics).to_a
-    allsynapses = Pundit.policy_scope(user, synapses).to_a
-    allmappings = Pundit.policy_scope(user, mappings).to_a
-
-    json = {}
-    json['map'] = self
-    json['topics'] = alltopics
-    json['synapses'] = allsynapses
-    json['mappings'] = allmappings
-    json['mappers'] = allmappers
-    json['collaborators'] = allcollaborators
-    json['messages'] = messages.sort_by(&:created_at)
-    json['stars'] = stars
+    {
+      map: self,
+      topics: Pundit.policy_scope(user, topics).to_a,
+      synapses: Pundit.policy_scope(user, synapses).to_a,
+      mappings: Pundit.policy_scope(user, mappings).to_a,
+      mappers: contributors,
+      collaborators: editors,
+      messages: messages.sort_by(&:created_at),
+      stars: stars
+    }
   end
 
   def add_new_collaborators(user_ids)
-    added = []
     users = User.where(id: user_ids)
-    users.each do |user|
-      if user && user != current_user && !collaborators.include?(user)
-        UserMap.create(user_id: uid.to_i, map_id: id)
-        user = User.find(uid.to_i)
-        added << user.id
-      end
+    current_collaborators = collaborators + [user]
+    added = users.map do |new_user|
+      next nil if current_collaborators.include?(new_user)
+      UserMap.create(user_id: new_user.id, map_id: id)
+      new_user.id
     end
-    added
+    added.compact
   end
 
   def remove_old_collaborators(user_ids)
-    removed = []
-    collaborators.map(&:id).each do |user_id|
-      if !user_ids.include?(user_id)
-        user_maps.select { |um| um.user_id == user_id }.each(&:destroy)
-        removed << user_id
-      end
+    current_collaborators = collaborators + [user]
+    removed = current_collaborators.map(&:id).map do |old_user_id|
+      next nil if user_ids.include?(old_user_id)
+      user_maps.where(user_id: old_user_id).find_each(&:destroy)
+      old_user_id
     end
-    removed
+    removed.compact
   end
 
   def base64_screenshot(encoded_image)

From dad048eb20341a6acd27bf2e72dd65b5c17e0809 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sat, 24 Sep 2016 12:58:09 +0800
Subject: [PATCH 09/13] rubocop

---
 app/controllers/explore_controller.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/controllers/explore_controller.rb b/app/controllers/explore_controller.rb
index 023a6866..3c099920 100644
--- a/app/controllers/explore_controller.rb
+++ b/app/controllers/explore_controller.rb
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 class ExploreController < ApplicationController
   before_action :authorize_explore
   after_action :verify_authorized
@@ -5,8 +6,8 @@ class ExploreController < ApplicationController
 
   respond_to :html, :json, :csv
 
-  # TODO remove?
-  #autocomplete :map, :name, full: true, extra_data: [:user_id]
+  # TODO: remove?
+  # autocomplete :map, :name, full: true, extra_data: [:user_id]
 
   # GET /explore/active
   def active

From 50f98aebea73fab221a4d0534fb07aef7da6b48b Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sat, 24 Sep 2016 14:35:23 +0800
Subject: [PATCH 10/13] explore controller spec

---
 app/controllers/explore_controller.rb       | 12 +++++------
 app/policies/explore_policy.rb              | 12 +++++------
 spec/controllers/explore_controller_spec.rb | 23 +++++++++++++++++++++
 spec/controllers/maps_controller_spec.rb    | 15 --------------
 4 files changed, 35 insertions(+), 27 deletions(-)
 create mode 100644 spec/controllers/explore_controller_spec.rb

diff --git a/app/controllers/explore_controller.rb b/app/controllers/explore_controller.rb
index 3c099920..6f24eba5 100644
--- a/app/controllers/explore_controller.rb
+++ b/app/controllers/explore_controller.rb
@@ -21,7 +21,7 @@ class ExploreController < ApplicationController
         redirect_to(root_url) && return if authenticated?
         respond_with(@maps, @user)
       end
-      format.json { render json: @maps }
+      format.json { render json: @maps.to_json }
     end
   end
 
@@ -35,7 +35,7 @@ class ExploreController < ApplicationController
 
     respond_to do |format|
       format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps }
+      format.json { render json: @maps.to_json }
     end
   end
 
@@ -53,7 +53,7 @@ class ExploreController < ApplicationController
 
     respond_to do |format|
       format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps }
+      format.json { render json: @maps.to_json }
     end
   end
 
@@ -71,7 +71,7 @@ class ExploreController < ApplicationController
 
     respond_to do |format|
       format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps }
+      format.json { render json: @maps.to_json }
     end
   end
 
@@ -90,7 +90,7 @@ class ExploreController < ApplicationController
 
     respond_to do |format|
       format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps }
+      format.json { render json: @maps.to_json }
     end
   end
 
@@ -103,7 +103,7 @@ class ExploreController < ApplicationController
 
     respond_to do |format|
       format.html { respond_with(@maps, @user) }
-      format.json { render json: @maps }
+      format.json { render json: @maps.to_json }
     end
   end
 
diff --git a/app/policies/explore_policy.rb b/app/policies/explore_policy.rb
index 6cbdab15..b4d52fe5 100644
--- a/app/policies/explore_policy.rb
+++ b/app/policies/explore_policy.rb
@@ -1,25 +1,25 @@
 class ExplorePolicy < ApplicationPolicy
-  def activemaps?
+  def active?
     true
   end
 
-  def featuredmaps?
+  def featured?
     true
   end
 
-  def mymaps?
+  def mine?
     true
   end
 
-  def sharedmaps?
+  def shared?
     true
   end
 
-  def starredmaps?
+  def starred?
     true
   end
 
-  def usermaps?
+  def mapper?
     true
   end
 end
diff --git a/spec/controllers/explore_controller_spec.rb b/spec/controllers/explore_controller_spec.rb
new file mode 100644
index 00000000..4e298a92
--- /dev/null
+++ b/spec/controllers/explore_controller_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+RSpec.describe ExploreController, type: :controller do
+  before :each do
+    sign_in create(:user)
+  end
+
+  describe 'GET explore/active' do
+    context 'always returns an array' do
+      it 'with 0 records' do
+        Map.delete_all
+        get :active, format: :json
+        expect(JSON.parse(response.body)).to eq []
+      end
+      it 'with 1 record' do
+        map = create(:map)
+        get :active, format: :json
+        expect(JSON.parse(response.body).class).to be Array
+      end
+    end
+  end
+end
diff --git a/spec/controllers/maps_controller_spec.rb b/spec/controllers/maps_controller_spec.rb
index b877dc88..0f053dd9 100644
--- a/spec/controllers/maps_controller_spec.rb
+++ b/spec/controllers/maps_controller_spec.rb
@@ -9,21 +9,6 @@ RSpec.describe MapsController, type: :controller do
     sign_in create(:user)
   end
 
-  describe 'GET #activemaps' do
-    context 'always returns an array' do
-      it 'with 0 records' do
-        Map.delete_all
-        get :activemaps, format: :json
-        expect(JSON.parse(response.body)).to eq []
-      end
-      it 'with 1 record' do
-        map = create(:map)
-        get :activemaps, format: :json
-        expect(JSON.parse(response.body).class).to be Array
-      end
-    end
-  end
-
   describe 'POST #create' do
     context 'with valid params' do
       it 'creates a new Map' do

From 18d8929bf159821d2231a4833c443b3eb8c571d7 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sun, 25 Sep 2016 23:35:26 +0800
Subject: [PATCH 11/13] use .or to fix all sorts of @map.mappings bugs

---
 app/models/map.rb | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/models/map.rb b/app/models/map.rb
index 0cab398d..f9fe6312 100644
--- a/app/models/map.rb
+++ b/app/models/map.rb
@@ -16,11 +16,12 @@ class Map < ApplicationRecord
   has_many :events, -> { includes :user }, as: :eventable, dependent: :destroy
 
   # This method associates the attribute ":image" with a file attachment
-  has_attached_file :screenshot, styles: {
-    thumb: ['188x126#', :png]
-    #:full => ['940x630#', :png]
-  },
-                                 default_url: 'https://s3.amazonaws.com/metamaps-assets/site/missing-map-white.png'
+  has_attached_file :screenshot,
+    styles: {
+      thumb: ['188x126#', :png]
+      #:full => ['940x630#', :png]
+    },
+    default_url: 'https://s3.amazonaws.com/metamaps-assets/site/missing-map-white.png'
 
   validates :name, presence: true
   validates :arranged, inclusion: { in: [true, false] }
@@ -31,7 +32,7 @@ class Map < ApplicationRecord
   validates_attachment_content_type :screenshot, content_type: /\Aimage\/.*\Z/
 
   def mappings
-    topicmappings + synapsemappings
+    topicmappings.or(synapsemappings)
   end
 
   def mk_permission

From 05495b02246afa85845e0c70b5668283295f2c50 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sun, 25 Sep 2016 23:35:35 +0800
Subject: [PATCH 12/13] move explore views to their own folder

---
 app/views/{maps/activemaps.html.erb => explore/active.html.erb}   | 0
 .../{maps/featuredmaps.html.erb => explore/featured.html.erb}     | 0
 app/views/{maps/usermaps.html.erb => explore/mapper.html.erb}     | 0
 app/views/{maps/mymaps.html.erb => explore/mine.html.erb}         | 0
 app/views/{maps/sharedmaps.html.erb => explore/shared.html.erb}   | 0
 app/views/{maps/starredmaps.html.erb => explore/starred.html.erb} | 0
 6 files changed, 0 insertions(+), 0 deletions(-)
 rename app/views/{maps/activemaps.html.erb => explore/active.html.erb} (100%)
 rename app/views/{maps/featuredmaps.html.erb => explore/featured.html.erb} (100%)
 rename app/views/{maps/usermaps.html.erb => explore/mapper.html.erb} (100%)
 rename app/views/{maps/mymaps.html.erb => explore/mine.html.erb} (100%)
 rename app/views/{maps/sharedmaps.html.erb => explore/shared.html.erb} (100%)
 rename app/views/{maps/starredmaps.html.erb => explore/starred.html.erb} (100%)

diff --git a/app/views/maps/activemaps.html.erb b/app/views/explore/active.html.erb
similarity index 100%
rename from app/views/maps/activemaps.html.erb
rename to app/views/explore/active.html.erb
diff --git a/app/views/maps/featuredmaps.html.erb b/app/views/explore/featured.html.erb
similarity index 100%
rename from app/views/maps/featuredmaps.html.erb
rename to app/views/explore/featured.html.erb
diff --git a/app/views/maps/usermaps.html.erb b/app/views/explore/mapper.html.erb
similarity index 100%
rename from app/views/maps/usermaps.html.erb
rename to app/views/explore/mapper.html.erb
diff --git a/app/views/maps/mymaps.html.erb b/app/views/explore/mine.html.erb
similarity index 100%
rename from app/views/maps/mymaps.html.erb
rename to app/views/explore/mine.html.erb
diff --git a/app/views/maps/sharedmaps.html.erb b/app/views/explore/shared.html.erb
similarity index 100%
rename from app/views/maps/sharedmaps.html.erb
rename to app/views/explore/shared.html.erb
diff --git a/app/views/maps/starredmaps.html.erb b/app/views/explore/starred.html.erb
similarity index 100%
rename from app/views/maps/starredmaps.html.erb
rename to app/views/explore/starred.html.erb

From 03ba3a89f1cf4c871f691d32fed7721fde605063 Mon Sep 17 00:00:00 2001
From: Devin Howard <devin@callysto.com>
Date: Sun, 25 Sep 2016 23:37:08 +0800
Subject: [PATCH 13/13] main controller renders by name

---
 app/controllers/main_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb
index 0d6af64b..4624c7a6 100644
--- a/app/controllers/main_controller.rb
+++ b/app/controllers/main_controller.rb
@@ -17,7 +17,7 @@ class MainController < ApplicationController
         if !authenticated?
           render 'main/home'
         else
-          render 'maps/activemaps'
+          render 'explore/active'
         end
       end
     end