From c9a74f28009c4caa510bf48740d4b8997d6384a8 Mon Sep 17 00:00:00 2001 From: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com> Date: Tue, 4 Feb 2025 14:09:10 -0800 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20reapply=20storage=20polic?= =?UTF-8?q?y=20errors=20out=20after=20a=20failed=20delete=20(#2674)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 reapply storage policy errors out after a failed delete * refactor: 💡 hide delete button when policy is retain forever * refactor: 💡 pr feedback (move check to model) --- addons/api/addon/adapters/application.js | 2 +- addons/api/addon/mixins/adapter-build-url.js | 4 +- addons/api/addon/models/session-recording.js | 10 ++- ui/admin/app/abilities/session-recording.js | 5 +- .../channels-by-connection/index.js | 11 ++- .../channels-by-connection/index.hbs | 7 +- .../unit/abilities/session-recording-test.js | 70 ++++++++++++++++++- 7 files changed, 93 insertions(+), 16 deletions(-) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 824b0fa0a..e0d930923 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -163,7 +163,7 @@ export default class ApplicationAdapter extends RESTAdapter.extend( } /** - * Overrides default ajax method by rewritting PUT requests as PATCH. + * Overrides default ajax method by rewriting PUT requests as PATCH. * PATCH is the request method used by our API for updates. * * @override diff --git a/addons/api/addon/mixins/adapter-build-url.js b/addons/api/addon/mixins/adapter-build-url.js index d595f5ff4..267e1cf30 100644 --- a/addons/api/addon/mixins/adapter-build-url.js +++ b/addons/api/addon/mixins/adapter-build-url.js @@ -6,7 +6,7 @@ import Mixin from '@ember/object/mixin'; /** - * Overrides URL-building methods in an adpater for greater flexibility. + * Overrides URL-building methods in an adapter for greater flexibility. * Specifically enhances calls to `urlPrefix` with `modelName`, `id`, and * `snapshot`, which aren't normally passed. And adds support for a * `urlSuffix` method, the return value of which is appended to the end of @@ -22,7 +22,7 @@ export default Mixin.create({ /** * This override behaves identically to the default method by default. - * It exists only to document the additional paramters to the method. + * It exists only to document the additional parameters to the method. * @override * @param {string} path * @param {string} parentURL diff --git a/addons/api/addon/models/session-recording.js b/addons/api/addon/models/session-recording.js index 49ee0ea58..aa930bdd0 100644 --- a/addons/api/addon/models/session-recording.js +++ b/addons/api/addon/models/session-recording.js @@ -71,7 +71,15 @@ export default class SessionRecordingModel extends GeneratedSessionRecordingMode } /** - * Reapply policy dates via the `reapply-staorage-policy` method. + * Returns true if retention of session-recording is forever. + * @type {boolean} + */ + get retainForever() { + return this.retain_until?.toISOString() === '9999-12-31T23:23:23.999Z'; + } + + /** + * Reapply policy dates via the `reapply-storage-policy` method. * @param {object} options * @param {object} options.adapterOptions * @return {Promise} diff --git a/ui/admin/app/abilities/session-recording.js b/ui/admin/app/abilities/session-recording.js index 85b4a3907..3e2779b88 100644 --- a/ui/admin/app/abilities/session-recording.js +++ b/ui/admin/app/abilities/session-recording.js @@ -32,10 +32,11 @@ export default class OverrideSessionRecordingAbility extends SessionRecordingAbi /** * This override ensures that session recordings may be deleted only if the - * session-recording feature flag is enabled. + * session-recording feature flag is enabled and policy allows deletion. */ get canDelete() { - return this.features.isEnabled('ssh-session-recording') + return this.features.isEnabled('ssh-session-recording') && + !this.model.retainForever ? super.canDelete : false; } diff --git a/ui/admin/app/controllers/scopes/scope/session-recordings/session-recording/channels-by-connection/index.js b/ui/admin/app/controllers/scopes/scope/session-recordings/session-recording/channels-by-connection/index.js index fd774c8ff..a0ac7360c 100644 --- a/ui/admin/app/controllers/scopes/scope/session-recordings/session-recording/channels-by-connection/index.js +++ b/ui/admin/app/controllers/scopes/scope/session-recordings/session-recording/channels-by-connection/index.js @@ -78,8 +78,13 @@ export default class ScopesScopeSessionRecordingsSessionRecordingChannelsByConne @notifyError(({ message }) => message, { catch: true }) @notifySuccess('notifications.delete-success') async delete(sessionRecording) { - await sessionRecording.destroyRecord(); - this.router.replaceWith('scopes.scope.session-recordings'); - this.router.refresh(); + try { + await sessionRecording.destroyRecord(); + this.router.replaceWith('scopes.scope.session-recordings'); + await this.router.refresh('scopes.scope.session-recordings'); + } catch (e) { + sessionRecording.rollbackAttributes(); + throw new Error(e); + } } } diff --git a/ui/admin/app/templates/scopes/scope/session-recordings/session-recording/channels-by-connection/index.hbs b/ui/admin/app/templates/scopes/scope/session-recordings/session-recording/channels-by-connection/index.hbs index 087858f31..a86a534d1 100644 --- a/ui/admin/app/templates/scopes/scope/session-recordings/session-recording/channels-by-connection/index.hbs +++ b/ui/admin/app/templates/scopes/scope/session-recordings/session-recording/channels-by-connection/index.hbs @@ -260,12 +260,7 @@ @model.sessionRecording.retain_until }} > - {{#if - (eq - (format-date-iso @model.sessionRecording.retain_until) - '9999-12-31T23:23:23.999Z' - ) - }} + {{#if @model.sessionRecording.retainForever}} {{t 'resources.policy.titles.forever'}} {{else}} {{format-date-iso @model.sessionRecording.retain_until}} diff --git a/ui/admin/tests/unit/abilities/session-recording-test.js b/ui/admin/tests/unit/abilities/session-recording-test.js index 6616b55ce..ab1bc819f 100644 --- a/ui/admin/tests/unit/abilities/session-recording-test.js +++ b/ui/admin/tests/unit/abilities/session-recording-test.js @@ -5,7 +5,11 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; -import { TYPE_SESSION_RECORDING_SSH } from 'api/models/session-recording'; +import { + TYPE_SESSION_RECORDING_SSH, + STATE_SESSION_RECORDING_AVAILABLE, +} from 'api/models/session-recording'; +import { faker } from '@faker-js/faker'; module('Unit | Abilities | session-recording', function (hooks) { setupTest(hooks); @@ -177,4 +181,68 @@ module('Unit | Abilities | session-recording', function (hooks) { }), ); }); + + test('can delete session recording when feature is enabled', async function (assert) { + features.enable('ssh-session-recording'); + const recordingWithAuthorizedAction = await store.createRecord( + 'session-recording', + { + authorized_actions: ['delete'], + type: TYPE_SESSION_RECORDING_SSH, + state: STATE_SESSION_RECORDING_AVAILABLE, + retain_until: faker.date.recent(), + }, + ); + const recordingWithoutAuthorizedAction = store.createRecord( + 'session-recording', + { + authorized_actions: [], + type: TYPE_SESSION_RECORDING_SSH, + state: STATE_SESSION_RECORDING_AVAILABLE, + retain_until: faker.date.recent(), + }, + ); + + assert.true( + canService.can('delete session-recording', recordingWithAuthorizedAction), + ); + assert.false( + canService.can( + 'delete session-recording', + recordingWithoutAuthorizedAction, + ), + ); + }); + + test('cannot delete session recording when feature is enabled but retain policy is forever', async function (assert) { + features.enable('ssh-session-recording'); + const recordingWithAuthorizedAction = await store.createRecord( + 'session-recording', + { + authorized_actions: ['delete'], + type: TYPE_SESSION_RECORDING_SSH, + state: STATE_SESSION_RECORDING_AVAILABLE, + retain_until: new Date('9999-12-31T23:23:23.999Z'), + }, + ); + const recordingWithoutAuthorizedAction = store.createRecord( + 'session-recording', + { + authorized_actions: [], + type: TYPE_SESSION_RECORDING_SSH, + state: STATE_SESSION_RECORDING_AVAILABLE, + retain_until: new Date('9999-12-31T23:23:23.999Z'), + }, + ); + + assert.false( + canService.can('delete session-recording', recordingWithAuthorizedAction), + ); + assert.false( + canService.can( + 'delete session-recording', + recordingWithoutAuthorizedAction, + ), + ); + }); });