whimsical-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s...@apache.org
Subject [whimsy] branch master updated: Simplify by passing expected and new phases from caller
Date Thu, 05 Jul 2018 00:39:06 GMT
This is an automated email from the ASF dual-hosted git repository.

sebb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/whimsy.git


The following commit(s) were added to refs/heads/master by this push:
     new c6e7284  Simplify by passing expected and new phases from caller
c6e7284 is described below

commit c6e72846f468775fd51edd57fd1331ba1293e9f7
Author: Sebb <sebb@apache.org>
AuthorDate: Thu Jul 5 01:39:05 2018 +0100

    Simplify by passing expected and new phases from caller
    
    Update code can be better controlled from caller
---
 www/project/icla/views/actions/update.json.rb | 113 ++++++++++++++++----------
 www/project/icla/views/pages/discuss.js.rb    |   9 +-
 www/project/icla/views/pages/vote.js.rb       |  15 ++--
 3 files changed, 84 insertions(+), 53 deletions(-)

diff --git a/www/project/icla/views/actions/update.json.rb b/www/project/icla/views/actions/update.json.rb
index 3a476c6..3267b58 100644
--- a/www/project/icla/views/actions/update.json.rb
+++ b/www/project/icla/views/actions/update.json.rb
@@ -1,6 +1,7 @@
 #
 # Common methods to update the progress file
-# TODO also send emails?
+#
+# Called from JS pages using POST
 #
 
 $LOAD_PATH.unshift '/srv/whimsy/lib'
@@ -8,24 +9,39 @@ $LOAD_PATH.unshift '/srv/whimsy/lib'
 require 'json'
 require 'whimsy/lockfile'
 
-# TODO add emails where necessary
 # TODO add some kind of history to show who changed the phase and when
 # This probably needs to be held separately from comments
 
+# Simplify validation
+VALID_ACTIONS=%w{submitVote cancelVote tallyVote submitComment startVoting invite}
+HAS_COMMENT=%w{submitComment startVoting invite} # do we update the comments array?
+VALID_PHASES=%w{discuss vote cancelled tallied invite}
+VALID_VOTES=%w{+1 +0 -0 -1}
+
 def update(data)
-  token = data['token'] 
-  file = "/srv/icla/#{token}.json"
+  # setup and validation
+  token = data['token']
+  raise ArgumentError.new('token must not be nil') unless token
   action = data['action']
-  timestamp = Time.now.to_s[0..9]
+  raise ArgumentError.new("Invalid action: '#{action}'") unless VALID_ACTIONS.include? action
   member = data['member']
   comment = data['comment'] # may be nil
+  expectedPhase = data['expectedPhase']
+  raise ArgumentError.new('expectedPhase must not be nil') unless expectedPhase
+  newPhase = data['newPhase'] # nil for no change
+  if newPhase and not VALID_PHASES.include? newPhase
+    raise ArgumentError.new("Invalid newPhase: '#{newPhase}'")
+  end
 
+  timestamp = Time.now.to_s[0..9]
+  addComment = nil
+  voteinfo = nil
   if action == 'submitVote'
     vote = data['vote']
-    raise 'vote must not be nil' unless vote
-    raise 'member must not be nil' unless member
+    raise ArgumentError.new("Invalid vote: '#{vote}'") unless VALID_VOTES.include? vote
+    raise ArgumentError.new('member must not be nil') unless member
     if vote == '-1'
-      raise '-1 vote must have comment' unless comment
+      raise ArgumentError.new('-1 vote must have comment') unless comment
     end
     if comment # allow comment for other votes
       voteinfo = {
@@ -41,42 +57,41 @@ def update(data)
         'timestamp' => timestamp,
       }
     end
+  elsif HAS_COMMENT.include? action
+    if comment
+      addComment = 
+      {
+        comment: comment,
+        member: member,
+        timestamp: timestamp,
+      } 
+    else
+      raise ArgumentError.new("comment must not be nil for '#{action}'")
+    end
   end
+
+  file = "/srv/icla/#{token}.json"
+
+  # now read/update the file if necessary
   contents = {} # define the var outside the block
+  rewrite = false # should the file be updated?
+  phases = *expectedPhase # convert string to array
+
   LockFile.lockfile(file, 'r+', File::LOCK_EX) do |f|
     contents = JSON::parse(f.read)
-    rewrite = false # should the file be updated?
-    case action
-      # These are the vote actions
-      when 'submitVote'
-        # keep the same phase
-        contents['votes'] << voteinfo
-        rewrite = true
-      when 'cancelVote'
-        contents['phase'] = 'cancelled'
-        rewrite = true
-      when 'tallyVote'
-        contents['phase'] = 'tallied' # is that necessary? Can we tally again?
-        rewrite = true # only needed if phase is updated
-
-      # these are the discuss actions
-      when 'submitComment', 'startVoting', 'invite' # discuss
-        contents['comments'] << {
-          comment: comment,
-          member: member,
-          timestamp: timestamp,
-        }
-        # Might be better for the caller to provide the new phase
-        if action == 'startVoting'
-          contents['phase'] = 'vote'
-          contents['votes'] ||= [] # make sure there is a votes array
-        end 
-        contents['phase'] = 'invite' if action == 'invite'
-        rewrite = true
-
-      # unknown
-      else
-        raise "InvalidAction: #{action}" 
+    phase = contents['phase']
+    raise ArgumentError.new("Phase '#{phase}': expected '#{expectedPhase}'") unless expectedPhase
== '*' or phases.include? phase 
+    if newPhase && newPhase != phase
+      contents['phase'] = newPhase
+      rewrite = true
+    end
+    if voteinfo
+      contents['votes'] << voteinfo
+      rewrite = true
+    end
+    if addComment
+      contents['comments'] << addComment
+      rewrite = true
     end
     if rewrite
       f.rewind # back to start
@@ -84,6 +99,9 @@ def update(data)
       f.write(JSON.pretty_generate(contents))
     end
   end
+
+  # return the data
+  _rewrite rewrite # debug
   contents
 end
 
@@ -93,7 +111,8 @@ def process(data)
   begin
     contents = update(data)
   rescue => e
-    _error e.inspect
+    _error e
+    _backtrace e.backtrace[0] # can be rather large
   end
   _contents contents
 end
@@ -108,10 +127,16 @@ end
 
 if __FILE__ == $0 # Allow independent testing
   $ret = {}
-  def method_missing(m, *args) # handles _error etc
-    $ret[m.to_s[1..-1]]=args[0] if m[0] == '_'
+  # method_missing caused some errors to be overlooked
+  %w{backtrace error contents rewrite}.each do |n|
+    define_method("_#{n}") do |a|
+      $ret[n] = a
+    end
   end
-  main(Hash[*ARGV])
+  data = Hash[*ARGV] # cannot combine this with next line as hash doesn't yet exist
+  data.each{|k,v| data[k] = v.split(',') if v =~ /,/} # fix up lists
+  puts data.inspect
+  main(data)
   puts JSON.pretty_generate($ret) # output the return data
 else
   embed # Sinatra sets params
diff --git a/www/project/icla/views/pages/discuss.js.rb b/www/project/icla/views/pages/discuss.js.rb
index 4ae2e77..e780a28 100644
--- a/www/project/icla/views/pages/discuss.js.rb
+++ b/www/project/icla/views/pages/discuss.js.rb
@@ -155,7 +155,6 @@ class Discuss < Vue
     checkValidity()
   end
 
-  # TODO
   def submitComment(event)
     console.log('submitComment discussBody: ' + @discussBody.inspect)
     updateVoteFile('submitComment')
@@ -163,20 +162,22 @@ class Discuss < Vue
 
   def startVoting(event)
     console.log('startVoting discussBody: ' + @discussBody.inspect)
-    updateVoteFile('startVoting')
+    updateVoteFile('startVoting','vote')
   end
 
-  def invite(event)
+  def invite(event) # does this need to change to 'invite'
     console.log('invite discussBody: ' + @discussBody.inspect)
     updateVoteFile('invite')
   end
 
-  def updateVoteFile(action)
+  def updateVoteFile(action, newPhase)
     data = {
       action: action,
       token: @token,
       member: @member,
       comment: @discussBody,
+      expectedPhase: 'discuss',
+      newPhase: newPhase,
     }
     console.log(">update: "+ data.inspect) # debug
     post 'update', data do |response|
diff --git a/www/project/icla/views/pages/vote.js.rb b/www/project/icla/views/pages/vote.js.rb
index 3e45d8b..37cd2d4 100644
--- a/www/project/icla/views/pages/vote.js.rb
+++ b/www/project/icla/views/pages/vote.js.rb
@@ -24,7 +24,7 @@ class Vote < Vue
       @iclaname = @contributor[:name]
       @iclaemail = @contributor[:email]
       @token = Server.data.token
-      @comments = @progress[:comments] ? @progress[:comments] : []
+      @comments = @progress[:comments]
       @votes = @progress[:votes]
       @vote = ''
       @timestamp = ''
@@ -184,27 +184,32 @@ class Vote < Vue
 
   def cancelVote(event)
     console.log('cancelVote:' + event)
-    updateVoteFile('cancelVote')
+    updateVoteFile('cancelVote', 'cancelled')
   end
 
   def tallyVote(event)
     console.log('tallyVote:' + event)
-    updateVoteFile('tallyVote')
+    updateVoteFile('tallyVote', 'tallied') # is this correct? TODO
   end
 
-  def updateVoteFile(action)
+  def updateVoteFile(action, newPhase)
     data = {
       action: action,
       vote: @vote,
       member: @member,
       token: @token,
+      expectedPhase: 'vote',
+      newPhase: newPhase,
     }
     data['comment']=@commentBody if @vote == '-1'
     console.log(">update: "+ data.inspect) # debug
     post 'update', data do |response|
       console.log("<update: "+ response.inspect) # debug
       @alert = response.error
-      @votes = response['contents']['votes'] unless @alert
+      unless @alert
+        @votes = response['contents']['votes']
+        @comments = response['contents']['comments']
+      end
     end
   end
 


Mime
View raw message