saxifrage / cityasacampus

An open-source platform for connecting and showcasing resources within local learning communities.
http://cityasacampus.org/
5 stars 4 forks source link

clean up pathways node managment #388

Closed chadwhitacre closed 3 years ago

chadwhitacre commented 8 years ago

Final PR for #364 (and a redo of #385). Builds on #377 and #384.

To-do

chadwhitacre commented 8 years ago

I'm looking to populate the nodes library with opportunities via the bulk uploader (#341). Using the template as ... a template, I'm getting errors with no errors:

screen shot 2015-12-30 at 3 02 49 pm

chadwhitacre commented 8 years ago

Errors for a single record as well.

chadwhitacre commented 8 years ago

Here it is:

screen shot 2015-12-30 at 3 14 07 pm

chadwhitacre commented 8 years ago

Running bundle exec rake db:seed fixed it.

chadwhitacre commented 8 years ago

The bulk uploader creates opportunities, but the map wants opportunity instances. :disappointed:

chadwhitacre commented 8 years ago

The silver spike!

chadwhitacre commented 8 years ago

the map wants opportunity instances

Or does it?

dmtroyer commented 8 years ago

At first thought I think it wants opportunities

On Wed, Dec 30, 2015, 16:06 Chad Whitacre notifications@github.com wrote:

the map wants opportunity instances

Or does it?

— Reply to this email directly or view it on GitHub https://github.com/saxifrage/cityasacampus/pull/388#issuecomment-168073789 .

chadwhitacre commented 8 years ago

@dmtroyer I seem to recall @timothyfcook saying the same. Makes things a little easier over here, so I will run with that. <:-)

chadwhitacre commented 8 years ago

screen shot 2015-12-30 at 7 52 43 pm

chadwhitacre commented 8 years ago

I'm looking at how to store the ordered list of nodes in the pathways. Currently we have a nodes table with this structure:

learn_dev=# \d nodes
                                        Table "public.nodes"
┌────────────────┬─────────────────────────────┬────────────────────────────────────────────────────┐
│     Column     │            Type             │                     Modifiers                      │
├────────────────┼─────────────────────────────┼────────────────────────────────────────────────────┤
│ id             │ integer                     │ not null default nextval('nodes_id_seq'::regclass) │
│ pathway_id     │ integer                     │ not null                                           │
│ opportunity_id │ integer                     │ not null                                           │
│ position       │ integer                     │ not null                                           │
│ created_at     │ timestamp without time zone │ not null                                           │
│ updated_at     │ timestamp without time zone │ not null                                           │
└────────────────┴─────────────────────────────┴────────────────────────────────────────────────────┘
Indexes:
    "nodes_pkey" PRIMARY KEY, btree (id)
    "index_nodes_on_opportunity_id" btree (opportunity_id)
    "index_nodes_on_pathway_id" btree (pathway_id)
Foreign-key constraints:
    "fk_rails_33dfa46f20" FOREIGN KEY (pathway_id) REFERENCES pathways(id)
    "fk_rails_cb5d35a32e" FOREIGN KEY (opportunity_id) REFERENCES opportunities(id)

learn_dev=#
dmtroyer commented 8 years ago

I'm looking at how to store the ordered list of nodes in the pathways.

Check out Pathway.nodes

dmtroyer commented 8 years ago

I guess that's retrieval of the ordered list, based on Pathway.position

chadwhitacre commented 8 years ago

I'm finding a suggestion to use large numbers for position, so that position can be changed for one node without having to touch the rest. This requires an occasional rebalancing as the ordering space gradually fills in.

chadwhitacre commented 8 years ago

use large numbers for position

Was that your intention with the nodes.position column?

chadwhitacre commented 8 years ago

Another pattern I find ("The Second Example") is to use a trigger to cascade position updates.

chadwhitacre commented 8 years ago

Another possibility would be to store an array of opportunity ids in the pathways table, but we don't get foreign key constraints in that case.

dmtroyer commented 8 years ago

Was that your intention with the nodes.position column?

I hadn't gotten that far in my thinking, but it makes sense. Probably whatever is easiest at this point.

chadwhitacre commented 8 years ago

Okay, I think I'm gonna try that ...

chadwhitacre commented 8 years ago

I have no idea why grids.json has started crapping out on me.

screen shot 2015-12-31 at 4 40 13 pm

I haven't touched that file. My stash:

diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb
index 008f35e..20b57b5 100644
--- a/app/controllers/api/nodes_controller.rb
+++ b/app/controllers/api/nodes_controller.rb
@@ -2,62 +2,27 @@ class Api::NodesController < ApplicationController
   before_action :authenticate_user!
   before_action :set_node, only: [:show, :edit, :update, :destroy]

-  # GET /nodes
-  # GET /nodes.json
-  def index
-    @nodes = Node.all
-
-    respond_to do |format|
-      format.html # index.html.erb
-      format.json { render json: @nodes }
-    end
-  end
-
-  # GET /nodes/1
-  # GET /nodes/1.json
-  def show
-    respond_to do |format|
-      format.html # show.html.erb
-      format.json { render json: @node }
-    end
-  end
-
-  # GET /nodes/new
-  def new
-    @node = Node.new
-  end
-
-  # GET /nodes/1/edit
-  def edit
-  end
-
   # POST /nodes
   # POST /nodes.json
   def create
-    @node = Node.new(node_params)

-    respond_to do |format|
-      if @node.save
-        format.html { redirect_to @node, notice: 'Node was successfully created.' }
-        format.json { render json: @node, status: :created }
-      else
-        format.html { render action: 'new' }
-        format.json { render json: @node.errors, status: :unprocessable_entity }
-      end
+    opportunity = Opportunity.find_by_id(node_params[:opportunity_id])!
+    if not current_user.organizers.include? opportunity.organizer
+      return render :plain => '{"error":"you don\'t have permission to edit this node"}', :status => 403
     end
-  end

-  # PATCH/PUT /nodes/1
-  # PATCH/PUT /nodes/1.json
-  def update
+    pathway = Pathway.find_by_id(node_params[:pathway_id])!
+  
+    begin
+      node = Node.where(opportunity_id: opportunity.id, pathway_id: pathway.id).take!
+      node.position = node_params[:position]
+    rescue
+      node = Node.new(node_params)
+    end
+    node.save
+
     respond_to do |format|
-      if @node.update(node_params)
-        format.html { redirect_to @node, notice: 'Node was successfully updated.' }
-        format.json { head :no_content }
-      else
-        format.html { render action: 'edit' }
-        format.json { render json: @node.errors, status: :unprocessable_entity }
-      end
+      format.json { head :no_content }
     end
   end

diff --git a/app/serializers/node_serializer.rb b/app/serializers/node_serializer.rb
index f2a9440..e891f02 100644
--- a/app/serializers/node_serializer.rb
+++ b/app/serializers/node_serializer.rb
@@ -1,3 +1,3 @@
 class NodeSerializer < ActiveModel::Serializer
-  attributes :id, :name
+  attributes :id, :name, :position, :pathway
 end
diff --git a/client/dashboard/organizers/map/MapController.js b/client/dashboard/organizers/map/MapController.js
index 2bec078..f20057e 100644
--- a/client/dashboard/organizers/map/MapController.js
+++ b/client/dashboard/organizers/map/MapController.js
@@ -97,8 +97,8 @@ angular.module('caac.dashboard.organizers.map.controller', [
     // Assignment
     // ==========

-    self.assignOrReorderNode = function(node) {
-        OpportunitiesService.assignOrReorder(node);
+    self.assignOrReorderNode = function(node, newIndex) {
+        OpportunitiesService.assignOrReorder(node.id, self.currentPathway.id, newIndex);
     };

     self.unassignNode = function(node) {
diff --git a/client/dashboard/organizers/map/MapView.html b/client/dashboard/organizers/map/MapView.html
index d76c49f..d43745e 100644
--- a/client/dashboard/organizers/map/MapView.html
+++ b/client/dashboard/organizers/map/MapView.html
@@ -32,7 +32,7 @@
   </div>
   <div class="pane dnd">
     <h3>Nodes in {{currentPathway.name||'...'}}</h3>
-    <ul dnd-list="currentPathway.nodes" dnd-drop="assignOrReorderNode(item)">
+    <ul dnd-list="currentPathway.nodes" dnd-drop="assignOrReorderNode(item, index)">
       <li ng-repeat="node in currentPathway.nodes"
           dnd-draggable="node"
           dnd-moved="currentPathway.nodes.splice($index, 1)"
diff --git a/client/opportunities/OpportunitiesService.js b/client/opportunities/OpportunitiesService.js
index 037df70..ed2ac83 100644
--- a/client/opportunities/OpportunitiesService.js
+++ b/client/opportunities/OpportunitiesService.js
@@ -8,12 +8,15 @@ angular.module('caac.opportunities.service', [
         return $http.get('/api/v1/opportunities.json?organizer_id='+organizerId);
       };

-      var assignOrReorder = function(node) {
-        $http.put('/api/v1/nodes/' + node.id + '.json')
+      var assignOrReorder = function(opportunityId, pathwayId, position) {
+        $http.post('/api/v1/nodes.json', { opportunity_id: opportunityId
+                                         , pathway_id: pathwayId
+                                         , position: position
+                                          });
       };

       var unassign = function(node) {
-        $http.put('/api/v1/nodes/' + node.id + '.json')
+        $http.delete('/api/v1/nodes/' + node.id + '.json')
       };

       return {
diff --git a/gulpfile.js b/gulpfile.js
index bd86bd4..ad56cfa 100644
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -39,7 +39,7 @@ gulp.task(tasks.JS_VENDORS, function() {
       appLocation + '/shared/modernizr/modernizr.js'
     ])
     .pipe(concat('vendors.min.js'))
-    .pipe(uglify())
+    //.pipe(uglify())
     .pipe(gulp.dest(buildLocation));
 });

@@ -51,7 +51,7 @@ gulp.task(tasks.JS_APP, function() {
     .pipe(sourcemaps.init())
     .pipe(concat('app.min.js'))
     .pipe(ngAnnotate())
-    .pipe(uglify())
+    //.pipe(uglify())
     .pipe(sourcemaps.write())
     .pipe(gulp.dest(buildLocation));
 });
chadwhitacre commented 8 years ago

The string name does not appear on line 26 of that file. Why is Rails pointing me to that line?

chadwhitacre commented 8 years ago

I give up, @timothyfcook. No #364 this year.