Skip to content

Implement publish notifications opt-out #9359

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

Merged
merged 8 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions app/controllers/settings/profile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import Controller from '@ember/controller';
import { action } from '@ember/object';
import { service } from '@ember/service';
import { tracked } from '@glimmer/tracking';

import { task } from 'ember-concurrency';

export default class extends Controller {
@service notifications;

@tracked publishNotifications;

@action handleNotificationsChange(event) {
this.publishNotifications = event.target.checked;
}

updateNotificationSettings = task(async () => {
try {
await this.model.user.updatePublishNotifications(this.publishNotifications);
} catch {
this.notifications.error(
'Something went wrong while updating your notification settings. Please try again later!',
);
}
});
}
12 changes: 12 additions & 0 deletions app/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default class User extends Model {
@attr avatar;
@attr url;
@attr kind;
@attr publish_notifications;

async stats() {
return await waitForPromise(apiAction(this, { method: 'GET', path: 'stats' }));
Expand All @@ -34,6 +35,17 @@ export default class User extends Model {
});
}

async updatePublishNotifications(enabled) {
await waitForPromise(apiAction(this, { method: 'PUT', data: { user: { publish_notifications: enabled } } }));

this.store.pushPayload({
user: {
id: this.id,
publish_notifications: enabled,
},
});
}

async resendVerificationEmail() {
return await waitForPromise(apiAction(this, { method: 'PUT', path: 'resend' }));
}
Expand Down
5 changes: 5 additions & 0 deletions app/routes/settings/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@ export default class ProfileSettingsRoute extends AuthenticatedRoute {
async model() {
return { user: this.session.currentUser };
}

setupController(controller, model) {
super.setupController(...arguments);
controller.publishNotifications = model.user.publish_notifications;
}
}
38 changes: 37 additions & 1 deletion app/styles/settings/profile.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,43 @@
}

.me-email {
margin-bottom: var(--space-s);
margin-bottom: var(--space-m);
display: flex;
flex-direction: column;
}

.notifications {
margin-bottom: var(--space-s);
}

.checkbox-input {
display: grid;
grid-template:
"checkbox label" auto
"- note" auto /
auto 1fr;
row-gap: var(--space-3xs);
column-gap: var(--space-xs);
}

.label {
grid-area: label;
font-weight: bold;
}

.note {
grid-area: note;
display: block;
font-size: 85%;
}

.buttons {
display: flex;
align-items: center;
gap: var(--space-2xs);
margin-top: var(--space-s);
}

.update-prefs-button {
composes: yellow-button small from '../shared/buttons.module.css';
}
33 changes: 33 additions & 0 deletions app/templates/settings/profile.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,37 @@
data-test-email-input
/>
</div>

<div local-class="notifications" data-test-notifications>
<h2>Notification Settings</h2>

<label local-class="checkbox-input">
<Input
@type="checkbox"
@checked={{this.publishNotifications}}
disabled={{this.updateNotificationSettings.isRunning}}
{{on "change" this.handleNotificationsChange}}
/>
<span local-class="label">Publish Notifications</span>
<span local-class="note">
Publish notifications are sent to your email address whenever new
versions of a crate that you own are published. These can be useful to
quickly detect compromised accounts or API tokens.
</span>
</label>

<div local-class="buttons">
<button
type="button"
local-class="update-prefs-button"
disabled={{this.updateNotificationSettings.isRunning}}
{{on "click" (perform this.updateNotificationSettings)}}
>
Update preferences
</button>
{{#if this.updateNotificationSettings.isRunning}}
<LoadingSpinner local-class="spinner" data-test-spinner />
{{/if}}
</div>
</div>
</SettingsPage>
72 changes: 72 additions & 0 deletions e2e/acceptance/publish-notifications.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { expect, test } from '@/e2e/helper';

test.describe('Acceptance | publish notifications', { tag: '@acceptance' }, () => {
test('unsubscribe and resubscribe', async ({ page, mirage }) => {
await mirage.addHook(server => {
let user = server.create('user');
globalThis.user = user;
authenticateAs(user);
});

await page.goto('/settings/profile');
await expect(page).toHaveURL('/settings/profile');
await expect(page.locator('[data-test-notifications] input[type=checkbox]')).toBeChecked();

await page.click('[data-test-notifications] input[type=checkbox]');
await expect(page.locator('[data-test-notifications] input[type=checkbox]')).not.toBeChecked();

await page.click('[data-test-notifications] button');
await page.waitForFunction(() => globalThis.user.reload().publishNotifications === false);

await page.click('[data-test-notifications] input[type=checkbox]');
await expect(page.locator('[data-test-notifications] input[type=checkbox]')).toBeChecked();

await page.click('[data-test-notifications] button');
await page.waitForFunction(() => globalThis.user.reload().publishNotifications === true);
});

test('loading state', async ({ page, mirage }) => {
await mirage.addHook(server => {
let user = server.create('user');
authenticateAs(user);
globalThis.user = user;

globalThis.deferred = require('rsvp').defer();
server.put('/api/v1/users/:user_id', globalThis.deferred.promise);
});

await page.goto('/settings/profile');
await expect(page).toHaveURL('/settings/profile');

await page.click('[data-test-notifications] input[type=checkbox]');
await page.click('[data-test-notifications] button');
await expect(page.locator('[data-test-notifications] [data-test-spinner]')).toBeVisible();
await expect(page.locator('[data-test-notifications] input[type=checkbox]')).toBeDisabled();
await expect(page.locator('[data-test-notifications] button')).toBeDisabled();

await page.evaluate(async () => globalThis.deferred.resolve());
await expect(page.locator('[data-test-notifications] [data-test-spinner]')).not.toBeVisible();
await expect(page.locator('[data-test-notification-message="error"]')).not.toBeVisible();
});

test('error state', async ({ page, mirage }) => {
await mirage.addHook(server => {
server.logging = true;
let user = server.create('user');
authenticateAs(user);
globalThis.user = user;

server.put('/api/v1/users/:user_id', '', 500);
});

await page.goto('/settings/profile');
await expect(page).toHaveURL('/settings/profile');

await page.click('[data-test-notifications] input[type=checkbox]');
await page.click('[data-test-notifications] button');
await expect(page.locator('[data-test-notifications] [data-test-spinner]')).not.toBeVisible();
await expect(page.locator('[data-test-notification-message="error"]')).toHaveText(
'Something went wrong while updating your notification settings. Please try again later!',
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE users DROP publish_notifications;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
alter table users
add column publish_notifications boolean not null default true;

comment on column users.publish_notifications is 'Whether or not the user wants to receive notifications when a package they own is published';
1 change: 1 addition & 0 deletions mirage/factories/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default Factory.extend({
emailVerified: null,
emailVerificationToken: null,
isAdmin: false,
publishNotifications: true,

afterCreate(model) {
if (model.emailVerified === null) {
Expand Down
23 changes: 15 additions & 8 deletions mirage/route-handlers/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,25 @@ export function register(server) {
}

let json = JSON.parse(request.requestBody);
if (!json || !json.user || !('email' in json.user)) {
if (!json || !json.user) {
return new Response(400, {}, { errors: [{ detail: 'invalid json request' }] });
}
if (!json.user.email) {
return new Response(400, {}, { errors: [{ detail: 'empty email rejected' }] });

if (json.user.publish_notifications !== undefined) {
user.update({ publishNotifications: json.user.publish_notifications });
}

user.update({
email: json.user.email,
emailVerified: false,
emailVerificationToken: 'secret123',
});
if (json.user.email !== undefined) {
if (!json.user.email) {
return new Response(400, {}, { errors: [{ detail: 'empty email rejected' }] });
}

user.update({
email: json.user.email,
emailVerified: false,
emailVerificationToken: 'secret123',
});
}

return { ok: true };
});
Expand Down
1 change: 1 addition & 0 deletions mirage/serializers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default BaseSerializer.extend({
delete hash.email;
delete hash.email_verified;
delete hash.is_admin;
delete hash.publish_notifications;
} else {
hash.email_verification_sent = hash.email_verified || Boolean(hash.email_verification_token);
}
Expand Down
Loading
Loading