New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a notification subscribtion to the single post view #5722

Merged
merged 1 commit into from Mar 13, 2015

Conversation

Projects
None yet
5 participants
@AugierLe42e
Copy link
Contributor

AugierLe42e commented Mar 2, 2015

notifsubscribtion

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Mar 2, 2015

Thanks to deal with that! I agree that it's more important to have it in the SPV than in the stream, because when you get a notification and think "Damn, I don't want to know about this stuff anymore", you have a link to the post, but not to the stream. So, it's hard to find it again in your stream, and to be able to shut off (or on) the notification in the SPV is a nice improvement. I would like to see the same icon used on both places though, so if you change it here, you would have to do it in the stream and in the header too.

@AugierLe42e AugierLe42e force-pushed the AugierLe42e:add-notif-unsubscribe-single-post-view branch from 98ccf51 to 4b2d5fd Mar 7, 2015


createParticipation: function (evt) {
if(evt) { evt.preventDefault(); }
that = this;

This comment has been minimized.

@houndci-bot

houndci-bot Mar 7, 2015

'that' is not defined.

if(evt) { evt.preventDefault(); }
that = this;
$.post(Routes.post_participation_path(this.model.get("id")), {}, function () {
that.model.set({participation: true});

This comment has been minimized.

@houndci-bot

houndci-bot Mar 7, 2015

'that' is not defined.

that = this;
$.post(Routes.post_participation_path(this.model.get("id")), {}, function () {
that.model.set({participation: true});
that.render();

This comment has been minimized.

@houndci-bot

houndci-bot Mar 7, 2015

'that' is not defined.


destroyParticipation: function (evt) {
if(evt) { evt.preventDefault(); }
that = this;

This comment has been minimized.

@houndci-bot

houndci-bot Mar 7, 2015

'that' is not defined.

if(evt) { evt.preventDefault(); }
that = this;
$.post(Routes.post_participation_path(this.model.get("id")), { _method: "delete" }, function () {
that.model.set({participation: false});

This comment has been minimized.

@houndci-bot

houndci-bot Mar 7, 2015

'that' is not defined.

that = this;
$.post(Routes.post_participation_path(this.model.get("id")), { _method: "delete" }, function () {
that.model.set({participation: false});
that.render();

This comment has been minimized.

@houndci-bot

houndci-bot Mar 7, 2015

'that' is not defined.

@AugierLe42e

This comment has been minimized.

Copy link
Contributor

AugierLe42e commented Mar 7, 2015

This part may need a refactor, but too complex for me.
Is it worth to fix Milou's comments ?

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Mar 7, 2015

Is it worth to fix Milou's comments ?

Always, not fixing them requires a config change instead.

@AugierLe42e AugierLe42e force-pushed the AugierLe42e:add-notif-unsubscribe-single-post-view branch from 4b2d5fd to 6e05f0a Mar 7, 2015

@AugierLe42e

This comment has been minimized.

Copy link
Contributor

AugierLe42e commented Mar 7, 2015

Doing it right now so.Someone to help me with cucumber ?

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Mar 7, 2015

Looks like one of the "random" failures.

@AugierLe42e AugierLe42e changed the title [WIP] Add a notification subscribtion to the single post view Add a notification subscribtion to the single post view Mar 10, 2015

@AugierLe42e AugierLe42e force-pushed the AugierLe42e:add-notif-unsubscribe-single-post-view branch from 6e05f0a to 7ef2eb3 Mar 13, 2015

@AugierLe42e

This comment has been minimized.

Copy link
Contributor

AugierLe42e commented Mar 13, 2015

Can be merged now.

@AugierLe42e AugierLe42e force-pushed the AugierLe42e:add-notif-unsubscribe-single-post-view branch 2 times, most recently from 55cc54f to 0887137 Mar 13, 2015

@svbergerem svbergerem merged commit 0887137 into diaspora:develop Mar 13, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound Hound has reviewed all the changes!

svbergerem pushed a commit that referenced this pull request Mar 13, 2015

Steffen van Bergerem
Merge pull request #5722 from AugierLe42e/add-notif-unsubscribe-singl…
…e-post-view

Add a notification subscribtion to the single post view

@svbergerem svbergerem added this to the next-major milestone Mar 13, 2015

@svbergerem

This comment has been minimized.

Copy link
Member

svbergerem commented Mar 13, 2015

Thank you!

@AugierLe42e AugierLe42e deleted the AugierLe42e:add-notif-unsubscribe-single-post-view branch Mar 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment