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 <[email protected]>
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
