On Fri, Dec 15, 2017 at 10:11 AM, sebb <seb...@gmail.com> wrote: > On 15 December 2017 at 14:15, Sam Ruby <ru...@intertwingly.net> wrote: >> On Fri, Dec 15, 2017 at 9:03 AM, sebb <seb...@gmail.com> wrote: >>> I added a check for an empty list of ids to the server code for (p)pmc >>> updates. >> >> Cool. Thanks! >> >>> I have since found that it is possible to trigger this. >> >> How? Shouldn't that be fixed? > > Add committer to PMC > immediately select committer for removal. > The problem disappears if the PMC screen is refreshed first. > > Yes, it should be fixed. > But the server still needs to validate input data so there still needs > to be a way to feedback to the user. > >>> However the result is that the user gets a server failure message, >>> which is not very helpful. >>> >>> It would be better to return an error to the user, but it's not clear >>> how to do this. >> >> The current code (example: >> https://github.com/apache/whimsy/blob/5e99628962852cab88283849d0a06c44f29e4bfa/www/roster/views/mixins/add.js.rb#L48), >> checks for a status of 200, and if so parses the json and emits an >> update event. >> >> An example of handing update events: >> https://github.com/apache/whimsy/blob/5e99628962852cab88283849d0a06c44f29e4bfa/www/roster/views/pmc/main.js.rb#L192 >> >> Non-200 status codes are currently handled via an alert, and currently >> only show the status code and text. > > But how can committee.json.rb change the status code and text? > I tried adding > > return {status: 201, statusText: 'end' } > > but that just is treated as success AFAICT.
My thought process: if the user shouldn't be able to do something, and can; then it is our bug not theirs. As such raise an exception on the server, that will cause the body to contain a stack traceback, and send that output to the console. If there is no reasonable way for the client to prevent it, then send back a successful (in the HTTP 200 sense) response, but have the data contain the message to be shown to the user. >> --- >> >> If the UI were changed so that sending an empty list "should not >> happen", then an alert would be sufficient. Adding the body of the >> failed response to the console.log would be helpful. >> >> Alternately, the code could emit a failed event, and the that could be >> handled separately. >> >> - Sam Ruby - Sam Ruby
