On 15 December 2017 at 15:45, Sam Ruby <ru...@intertwingly.net> wrote: > 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.
That will require special processing on the client, because it will have to treat some responses as alerts. There is already such processing for status != 200. >>> --- >>> >>> 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
