Skip to content

Commit

Permalink
mixup! WIP undoable server updates.
Browse files Browse the repository at this point in the history
  • Loading branch information
jsa committed Aug 11, 2014
1 parent 62c70c1 commit 1a5bb04
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 12 deletions.
49 changes: 41 additions & 8 deletions common/static/coffee/src/discussion/views/discussion_content_view.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ if Backbone?

initialize: ->
@model.bind('change', @renderPartialAttrs, @)
@listenTo(@model, "change:endorsed", (model, value, options) =>
if @model instanceof Comment
@trigger("comment:endorse", model, value, options)
)

class @DiscussionContentShowView extends DiscussionContentView
events:
Expand Down Expand Up @@ -114,10 +118,16 @@ if Backbone?
event.preventDefault()
is_subscribed = @model.get("subscribed")
url = @model.urlFor(if is_subscribed then "unfollow" else "follow")
errorFunc = () =>
if not is_subscribed
msg = gettext("We had some trouble subscribing you to this thread. Please try again.")
else
msg = gettext("We had some trouble unsubscribing you from this thread. Please try again.")
DiscussionUtil.discussionAlert(gettext("Sorry"), msg)
DiscussionUtil.updateWithUndo(
@model,
{"subscribed": not is_subscribed},
{url: url, type: "POST", $elem: $(event.currentTarget)}
{url: url, type: "POST", error: errorFunc, $elem: $(event.currentTarget)}
)

toggleEndorse: (event) =>
Expand All @@ -129,11 +139,16 @@ if Backbone?
updates =
endorsed: not is_endorsed
endorsement: if is_endorsed then null else {username: DiscussionUtil.getUser().get("username"), time: new Date().toISOString()}
errorFunc = () =>
DiscussionUtil.discussionAlert(
gettext("Sorry"),
gettext("We had some trouble updating this thread. Please try again.")

This comment has been minimized.

Sign in to view

Copy link
@gwprice

gwprice Aug 11, 2014

Comments have a reference to the thread, so we should be able to choose a more specific message here.
)
DiscussionUtil.updateWithUndo(
@model,
updates,
{url: url, type: "POST", data: {endorsed: not is_endorsed}, $elem: $(event.currentTarget)},
).done(() => @trigger "comment:endorse", not is_endorsed)
{url: url, type: "POST", data: {endorsed: not is_endorsed}, error: errorFunc, $elem: $(event.currentTarget)},
)

toggleVote: (event) =>
event.preventDefault()
Expand All @@ -142,18 +157,23 @@ if Backbone?
url = @model.urlFor(if did_vote then "unvote" else "upvote")
updates =
upvoted_ids: (if did_vote then _.difference else _.union)(user.get('upvoted_ids'), [@model.id])
errorFunc = () =>
DiscussionUtil.discussionAlert(
gettext("Sorry"),
gettext("We had some trouble saving your vote. Please try again.")
)
DiscussionUtil.updateWithUndo(
user,
updates,
{url: url, type: "POST", $elem: $(event.currentTarget)},
{url: url, type: "POST", error: errorFunc, $elem: $(event.currentTarget)},
).done(() => if did_vote then @model.unvote() else @model.vote())

togglePin: (event) =>
event.preventDefault()
is_pinned = @model.get("pinned")
url = @model.urlFor(if is_pinned then "unPinThread" else "pinThread")
errorFunc = () =>
if newPinned
if not is_pinned
msg = gettext("We had some trouble pinning this thread. Please try again.")
else
msg = gettext("We had some trouble unpinning this thread. Please try again.")
Expand All @@ -174,16 +194,29 @@ if Backbone?
url = @model.urlFor(if is_flagged then "unFlagAbuse" else "flagAbuse")
updates =
abuse_flaggers: (if is_flagged then _.difference else _.union)(@model.get("abuse_flaggers"), [user.id])
errorFunc = () =>
DiscussionUtil.discussionAlert(
gettext("Sorry"),
gettext("We had some trouble updating this thread. Please try again.")
)
DiscussionUtil.updateWithUndo(
@model,
updates,
{url: url, type: "POST", $elem: $(event.currentTarget)},
{url: url, type: "POST", error: errorFunc, $elem: $(event.currentTarget)},
)

toggleClose: (event) =>
event.preventDefault()
updates =
closed: not @model.get('closed')
errorFunc = () =>
if not @model.get('closed')
msg = gettext("We had some trouble closing this thread. Please try again.")
else
msg = gettext("We had some trouble reopening this thread. Please try again.")
DiscussionUtil.discussionAlert(gettext("Sorry"), msg)
DiscussionUtil.updateWithUndo(
@model,
{closed: not @model.get('closed')},
{url: @model.urlFor("close"), type: "POST", $elem: $(event.currentTarget)},
updates,
{url: @model.urlFor("close"), type: "POST", data: updates, error: errorFunc, $elem: $(event.currentTarget)},
)
6 changes: 3 additions & 3 deletions common/static/coffee/src/discussion/views/discussion_thread_view.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ if Backbone?
response.set('thread', @model)
view = new ThreadResponseView($.extend({model: response}, options))
view.on "comment:add", @addComment
view.on "comment:endorse", @endorseThread
view.on "comment:endorse", (model, value, options) =>
@endorseThread(value)
view.render()
@$el.find(listSelector).append(view.el)
view.afterInsert()
Expand All @@ -221,8 +222,7 @@ if Backbone?
addComment: =>
@model.comment()

endorseThread: (endorsed) =>
is_endorsed = @$el.find(".action-answer.is-checked").length > 0
endorseThread: (is_endorsed) =>

This comment has been minimized.

Sign in to view

Copy link
@gwprice

gwprice Aug 11, 2014

This actually needs to stay as it was. The problem here is that the value that's being passed is the endorsed value of a particular response. In the event that an endorsement is being removed, we have to determine whether there is any other endorsed response.

This comment has been minimized.

Sign in to view

Copy link
@gwprice

gwprice Aug 11, 2014

Actually, not quite as it was; the parameter should just be removed (and call sites should be updated accordingly). We may also want to consider changing the name.

This comment has been minimized.

Sign in to view

Copy link
@jimabramson

jimabramson Aug 11, 2014

two problems:

  1. seems backwards to be using the DOM to determine the state of the model. (that was the original motivation for changing.)
  2. not necessarily a given that all a thread's endorsed responses will be in memory/onscreen at the moment one response's state is toggled, is it?

This comment has been minimized.

Sign in to view

Copy link
@gwprice

gwprice Aug 11, 2014

  1. I agree; sadly, the thread model does not currently have references to its responses.
  2. It is the case for questions that all endorsed responses will be onscreen, and practically speaking, only questions matter for this purpose given the current thread list UI.

This comment has been minimized.

Sign in to view

Copy link
@gwprice

gwprice Aug 11, 2014

(This is the reason that I originally had the endorsed count in the model in the CS)

This comment has been minimized.

Sign in to view

Copy link
@jimabramson

jimabramson Aug 11, 2014

ah. well - problem #3 is that the trigger event is always firing before the DOM has been updated, causing the sidebar indicator to display the opposite result of the one that's intended. deferring until the ajax operation resolved, as i had it originally, avoids this problem - i'm going to revert to that unless you have any other suggestions.

This comment has been minimized.

Sign in to view

Copy link
@gwprice

gwprice Aug 11, 2014

One hack that would at least keep things in the model would be to set endorsed_response_count on question threads when the responses are loaded and then adjust that number and update the endorsed boolean accordingly in this function.
@model.set 'endorsed', is_endorsed

submitComment: (event) ->
Expand Down
3 changes: 2 additions & 1 deletion common/static/coffee/src/discussion/views/thread_response_view.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ if Backbone?
@showView = new ThreadResponseShowView(model: @model)
@showView.bind "response:_delete", @_delete
@showView.bind "response:edit", @edit
@showView.on "comment:endorse", => @trigger("comment:endorse")
@showView.on "comment:endorse", (model, value, options) =>
@trigger("comment:endorse", model, value, options)

renderShowView: () ->
@renderSubView(@showView)
Expand Down

0 comments on commit 1a5bb04

Please sign in to comment.