Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up pathways node managment #388

Closed
wants to merge 5 commits into from
Closed

Conversation

chadwhitacre
Copy link
Contributor

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

To-do

  • constrain "Library" list by organizer and not-already-on-a-pathway-for-the-given-workflow-state
  • add/remove resources from $Pathway

@chadwhitacre
Copy link
Contributor Author

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
Copy link
Contributor Author

Errors for a single record as well.

@chadwhitacre
Copy link
Contributor Author

Here it is:

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

@chadwhitacre
Copy link
Contributor Author

Running bundle exec rake db:seed fixed it.

@chadwhitacre
Copy link
Contributor Author

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

@chadwhitacre
Copy link
Contributor Author

The silver spike!

@chadwhitacre
Copy link
Contributor Author

the map wants opportunity instances

Or does it?

@dmtroyer
Copy link
Contributor

At first thought I think it wants opportunities

On Wed, Dec 30, 2015, 16:06 Chad Whitacre [email protected] wrote:

the map wants opportunity instances

Or does it?


Reply to this email directly or view it on GitHub
#388 (comment)
.

@chadwhitacre
Copy link
Contributor Author

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

@chadwhitacre
Copy link
Contributor Author

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

@chadwhitacre
Copy link
Contributor Author

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             │ integernot null default nextval('nodes_id_seq'::regclass) │
│ pathway_id     │ integernot null                                           │
│ opportunity_id │ integernot null                                           │
│ position       │ integernot null                                           │
│ created_at     │ timestamp without time zonenot null                                           │
│ updated_at     │ timestamp without time zonenot 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
Copy link
Contributor

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

Check out Pathway.nodes

@dmtroyer
Copy link
Contributor

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

@chadwhitacre
Copy link
Contributor Author

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
Copy link
Contributor Author

use large numbers for position

Was that your intention with the nodes.position column?

@chadwhitacre
Copy link
Contributor Author

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

@chadwhitacre
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@chadwhitacre
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@chadwhitacre
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants