From cd566a944aae8d4c79c635a8694b0b1ed1b5dbb3 Mon Sep 17 00:00:00 2001
From: Alex Pennells <apennells@gitlab.com>
Date: Tue, 18 Oct 2022 17:09:48 +0000
Subject: [PATCH] Convert broadcast messages table to Vue
Builds out a broadcast messages table in Vue that is
equivalent to the existing HAML table. The Vue table is still
locked behind the vue_broadcast_messages feature flag.
---
.../broadcast_messages/components/base.vue | 93 +++++++++++++++-
.../components/messages_table.vue | 102 +++++++++++++++++-
.../components/messages_table_row.vue | 16 ---
.../admin/broadcast_messages/index.js | 6 +-
.../admin/broadcast_messages_controller.rb | 1 +
.../admin/broadcast_messages/index.html.haml | 17 ++-
locale/gitlab.pot | 3 +
.../components/base_spec.js | 83 +++++++++++++-
.../components/messages_table_row_spec.js | 26 -----
.../components/messages_table_spec.js | 32 +++++-
.../admin/broadcast_messages/mock_data.js | 20 +++-
11 files changed, 336 insertions(+), 63 deletions(-)
delete mode 100644 app/assets/javascripts/admin/broadcast_messages/components/messages_table_row.vue
delete mode 100644 spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js
diff --git a/app/assets/javascripts/admin/broadcast_messages/components/base.vue b/app/assets/javascripts/admin/broadcast_messages/components/base.vue
index bc395a836257c..b7bafe46327ab 100644
--- a/app/assets/javascripts/admin/broadcast_messages/components/base.vue
+++ b/app/assets/javascripts/admin/broadcast_messages/components/base.vue
@@ -1,21 +1,112 @@
<script>
+import { GlPagination } from '@gitlab/ui';
+import { redirectTo } from '~/lib/utils/url_utility';
+import { buildUrlWithCurrentLocation } from '~/lib/utils/common_utils';
+import { createAlert, VARIANT_DANGER } from '~/flash';
+import { s__ } from '~/locale';
+import axios from '~/lib/utils/axios_utils';
import MessagesTable from './messages_table.vue';
+const PER_PAGE = 20;
+
export default {
name: 'BroadcastMessagesBase',
components: {
+ GlPagination,
MessagesTable,
},
+
props: {
+ page: {
+ type: Number,
+ required: true,
+ },
+ messagesCount: {
+ type: Number,
+ required: true,
+ },
messages: {
type: Array,
required: true,
},
},
+
+ i18n: {
+ deleteError: s__(
+ 'BroadcastMessages|There was an issue deleting this message, please try again later.',
+ ),
+ },
+
+ data() {
+ return {
+ currentPage: this.page,
+ totalMessages: this.messagesCount,
+ visibleMessages: this.messages.map((message) => ({
+ ...message,
+ disable_delete: false,
+ })),
+ };
+ },
+
+ computed: {
+ hasVisibleMessages() {
+ return this.visibleMessages.length > 0;
+ },
+ },
+
+ watch: {
+ totalMessages(newVal, oldVal) {
+ // Pagination controls disappear when there is only
+ // one page worth of messages. Since we're relying on static data,
+ // this could hide messages on the next page, or leave the user
+ // stranded on page 2 when deleting the last message.
+ // Force a page reload to avoid this edge case.
+ if (newVal === PER_PAGE && oldVal === PER_PAGE + 1) {
+ redirectTo(this.buildPageUrl(1));
+ }
+ },
+ },
+
+ methods: {
+ buildPageUrl(newPage) {
+ return buildUrlWithCurrentLocation(`?page=${newPage}`);
+ },
+
+ async deleteMessage(messageId) {
+ const index = this.visibleMessages.findIndex((m) => m.id === messageId);
+ if (!index === -1) return;
+
+ const message = this.visibleMessages[index];
+ this.$set(this.visibleMessages, index, { ...message, disable_delete: true });
+
+ try {
+ await axios.delete(message.delete_path);
+ } catch (e) {
+ this.$set(this.visibleMessages, index, { ...message, disable_delete: false });
+ createAlert({ message: this.$options.i18n.deleteError, variant: VARIANT_DANGER });
+ return;
+ }
+
+ // Remove the message from the table
+ this.visibleMessages = this.visibleMessages.filter((m) => m.id !== messageId);
+ this.totalMessages -= 1;
+ },
+ },
};
</script>
+
<template>
<div>
- <messages-table v-if="messages.length > 0" :messages="messages" />
+ <messages-table
+ v-if="hasVisibleMessages"
+ :messages="visibleMessages"
+ @delete-message="deleteMessage"
+ />
+ <gl-pagination
+ v-model="currentPage"
+ :total-items="totalMessages"
+ :link-gen="buildPageUrl"
+ align="center"
+ />
</div>
</template>
diff --git a/app/assets/javascripts/admin/broadcast_messages/components/messages_table.vue b/app/assets/javascripts/admin/broadcast_messages/components/messages_table.vue
index 7b531b850c617..1408312d3e433 100644
--- a/app/assets/javascripts/admin/broadcast_messages/components/messages_table.vue
+++ b/app/assets/javascripts/admin/broadcast_messages/components/messages_table.vue
@@ -1,10 +1,23 @@
<script>
-import MessagesTableRow from './messages_table_row.vue';
+import { GlButton, GlTableLite, GlSafeHtmlDirective } from '@gitlab/ui';
+import { __ } from '~/locale';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+
+const DEFAULT_TD_CLASSES = 'gl-vertical-align-middle!';
export default {
name: 'MessagesTable',
components: {
- MessagesTableRow,
+ GlButton,
+ GlTableLite,
+ },
+ directives: {
+ SafeHtml: GlSafeHtmlDirective,
+ },
+ mixins: [glFeatureFlagsMixin()],
+ i18n: {
+ edit: __('Edit'),
+ delete: __('Delete'),
},
props: {
messages: {
@@ -12,10 +25,89 @@ export default {
required: true,
},
},
+ computed: {
+ fields() {
+ if (this.glFeatures.roleTargetedBroadcastMessages) return this.$options.allFields;
+ return this.$options.allFields.filter((f) => f.key !== 'target_roles');
+ },
+ },
+ allFields: [
+ {
+ key: 'status',
+ label: __('Status'),
+ tdClass: DEFAULT_TD_CLASSES,
+ },
+ {
+ key: 'preview',
+ label: __('Preview'),
+ tdClass: DEFAULT_TD_CLASSES,
+ },
+ {
+ key: 'starts_at',
+ label: __('Starts'),
+ tdClass: DEFAULT_TD_CLASSES,
+ },
+ {
+ key: 'ends_at',
+ label: __('Ends'),
+ tdClass: DEFAULT_TD_CLASSES,
+ },
+ {
+ key: 'target_roles',
+ label: __('Target roles'),
+ tdClass: DEFAULT_TD_CLASSES,
+ thAttr: { 'data-testid': 'target-roles-th' },
+ },
+ {
+ key: 'target_path',
+ label: __('Target Path'),
+ tdClass: DEFAULT_TD_CLASSES,
+ },
+ {
+ key: 'type',
+ label: __('Type'),
+ tdClass: DEFAULT_TD_CLASSES,
+ },
+ {
+ key: 'buttons',
+ label: '',
+ tdClass: `${DEFAULT_TD_CLASSES} gl-white-space-nowrap`,
+ },
+ ],
+ safeHtmlConfig: {
+ ADD_TAGS: ['use'],
+ },
};
</script>
<template>
- <div>
- <messages-table-row v-for="message in messages" :key="message.id" :message="message" />
- </div>
+ <gl-table-lite
+ :items="messages"
+ :fields="fields"
+ :tbody-tr-attr="{ 'data-testid': 'message-row' }"
+ stacked="md"
+ >
+ <template #cell(preview)="{ item: { preview } }">
+ <div v-safe-html:[$options.safeHtmlConfig]="preview"></div>
+ </template>
+
+ <template #cell(buttons)="{ item: { id, edit_path, disable_delete } }">
+ <gl-button
+ icon="pencil"
+ :aria-label="$options.i18n.edit"
+ :href="edit_path"
+ data-testid="edit-message"
+ />
+
+ <gl-button
+ class="gl-ml-3"
+ icon="remove"
+ variant="danger"
+ :aria-label="$options.i18n.delete"
+ rel="nofollow"
+ :disabled="disable_delete"
+ :data-testid="`delete-message-${id}`"
+ @click="$emit('delete-message', id)"
+ />
+ </template>
+ </gl-table-lite>
</template>
diff --git a/app/assets/javascripts/admin/broadcast_messages/components/messages_table_row.vue b/app/assets/javascripts/admin/broadcast_messages/components/messages_table_row.vue
deleted file mode 100644
index bd45bcc4fc422..0000000000000
--- a/app/assets/javascripts/admin/broadcast_messages/components/messages_table_row.vue
+++ /dev/null
@@ -1,16 +0,0 @@
-<script>
-export default {
- name: 'MessagesTableRow',
- props: {
- message: {
- type: Object,
- required: true,
- },
- },
-};
-</script>
-<template>
- <div>
- {{ message.id }}
- </div>
-</template>
diff --git a/app/assets/javascripts/admin/broadcast_messages/index.js b/app/assets/javascripts/admin/broadcast_messages/index.js
index e71495804ee55..81952d2033ee0 100644
--- a/app/assets/javascripts/admin/broadcast_messages/index.js
+++ b/app/assets/javascripts/admin/broadcast_messages/index.js
@@ -3,14 +3,16 @@ import BroadcastMessagesBase from './components/base.vue';
export default () => {
const el = document.querySelector('#js-broadcast-messages');
- const { messages } = el.dataset;
+ const { page, messagesCount, messages } = el.dataset;
return new Vue({
el,
- name: 'BroadcastMessagesBase',
+ name: 'BroadcastMessages',
render(createElement) {
return createElement(BroadcastMessagesBase, {
props: {
+ page: Number(page),
+ messagesCount: Number(messagesCount),
messages: JSON.parse(messages),
},
});
diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb
index 4159e4b206fbb..edd8541469662 100644
--- a/app/controllers/admin/broadcast_messages_controller.rb
+++ b/app/controllers/admin/broadcast_messages_controller.rb
@@ -11,6 +11,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
# rubocop: disable CodeReuse/ActiveRecord
def index
push_frontend_feature_flag(:vue_broadcast_messages, current_user)
+ push_frontend_feature_flag(:role_targeted_broadcast_messages, current_user)
@broadcast_messages = BroadcastMessage.order(ends_at: :desc).page(params[:page])
@broadcast_message = BroadcastMessage.new
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 6f00b3d68fe1c..7559365e49a91 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -8,7 +8,22 @@
= _('Use banners and notifications to notify your users about scheduled maintenance, recent upgrades, and more.')
- if vue_app_enabled
- #js-broadcast-messages{ data: { messages: @broadcast_messages.to_json } }
+ #js-broadcast-messages{ data: {
+ page: params[:page] || 1,
+ messages_count: @broadcast_messages.total_count,
+ messages: @broadcast_messages.map { |message| {
+ id: message.id,
+ status: broadcast_message_status(message),
+ preview: broadcast_message(message, preview: true),
+ starts_at: message.starts_at.to_s,
+ ends_at: message.ends_at.to_s,
+ target_roles: target_access_levels_display(message.target_access_levels),
+ target_path: message.target_path,
+ type: message.broadcast_type.capitalize,
+ edit_path: edit_admin_broadcast_message_path(message),
+ delete_path: admin_broadcast_message_path(message) + '.js'
+ } }.to_json
+ } }
- else
= render 'form'
%br.clearfix
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a4ac7b42231a7..39259597a6c1f 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -7056,6 +7056,9 @@ msgstr ""
msgid "Broadcast Messages"
msgstr ""
+msgid "BroadcastMessages|There was an issue deleting this message, please try again later."
+msgstr ""
+
msgid "Browse Directory"
msgstr ""
diff --git a/spec/frontend/admin/broadcast_messages/components/base_spec.js b/spec/frontend/admin/broadcast_messages/components/base_spec.js
index 65f828ff6b45e..020e1c1d7c14c 100644
--- a/spec/frontend/admin/broadcast_messages/components/base_spec.js
+++ b/spec/frontend/admin/broadcast_messages/components/base_spec.js
@@ -1,35 +1,112 @@
import { shallowMount } from '@vue/test-utils';
+import { GlPagination } from '@gitlab/ui';
+import AxiosMockAdapter from 'axios-mock-adapter';
+import { TEST_HOST } from 'helpers/test_constants';
+import waitForPromises from 'helpers/wait_for_promises';
+import { useMockLocationHelper } from 'helpers/mock_window_location_helper';
+import { createAlert } from '~/flash';
+import axios from '~/lib/utils/axios_utils';
+import { redirectTo } from '~/lib/utils/url_utility';
import BroadcastMessagesBase from '~/admin/broadcast_messages/components/base.vue';
import MessagesTable from '~/admin/broadcast_messages/components/messages_table.vue';
-import { MOCK_MESSAGES } from '../mock_data';
+import { generateMockMessages, MOCK_MESSAGES } from '../mock_data';
+
+jest.mock('~/flash');
+jest.mock('~/lib/utils/url_utility');
describe('BroadcastMessagesBase', () => {
let wrapper;
+ let axiosMock;
+
+ useMockLocationHelper();
const findTable = () => wrapper.findComponent(MessagesTable);
+ const findPagination = () => wrapper.findComponent(GlPagination);
function createComponent(props = {}) {
wrapper = shallowMount(BroadcastMessagesBase, {
propsData: {
+ page: 1,
+ messagesCount: MOCK_MESSAGES.length,
messages: MOCK_MESSAGES,
...props,
},
});
}
+ beforeEach(() => {
+ axiosMock = new AxiosMockAdapter(axios);
+ });
+
afterEach(() => {
+ axiosMock.restore();
wrapper.destroy();
});
- it('renders the table when there are existing messages', () => {
+ it('renders the table and pagination when there are existing messages', () => {
createComponent();
expect(findTable().exists()).toBe(true);
+ expect(findPagination().exists()).toBe(true);
});
- it('does not render the table when there are no existing messages', () => {
+ it('does not render the table when there are no visible messages', () => {
createComponent({ messages: [] });
expect(findTable().exists()).toBe(false);
+ expect(findPagination().exists()).toBe(true);
+ });
+
+ it('does not remove a deleted message if it was not in visibleMessages', async () => {
+ createComponent();
+
+ findTable().vm.$emit('delete-message', -1);
+ await waitForPromises();
+
+ expect(axiosMock.history.delete).toHaveLength(0);
+ expect(wrapper.vm.visibleMessages.length).toBe(MOCK_MESSAGES.length);
+ });
+
+ it('does not remove a deleted message if the request fails', async () => {
+ createComponent();
+ const { id, delete_path } = MOCK_MESSAGES[0];
+ axiosMock.onDelete(delete_path).replyOnce(500);
+
+ findTable().vm.$emit('delete-message', id);
+ await waitForPromises();
+
+ expect(wrapper.vm.visibleMessages.find((m) => m.id === id)).not.toBeUndefined();
+ expect(createAlert).toHaveBeenCalledWith(
+ expect.objectContaining({
+ message: BroadcastMessagesBase.i18n.deleteError,
+ }),
+ );
+ });
+
+ it('removes a deleted message from visibleMessages on success', async () => {
+ createComponent();
+ const { id, delete_path } = MOCK_MESSAGES[0];
+ axiosMock.onDelete(delete_path).replyOnce(200);
+
+ findTable().vm.$emit('delete-message', id);
+ await waitForPromises();
+
+ expect(wrapper.vm.visibleMessages.find((m) => m.id === id)).toBeUndefined();
+ expect(wrapper.vm.totalMessages).toBe(MOCK_MESSAGES.length - 1);
+ });
+
+ it('redirects to the first page when totalMessages changes from 21 to 20', async () => {
+ window.location.pathname = `${TEST_HOST}/admin/broadcast_messages`;
+
+ const messages = generateMockMessages(21);
+ const { id, delete_path } = messages[0];
+ createComponent({ messages, messagesCount: messages.length });
+
+ axiosMock.onDelete(delete_path).replyOnce(200);
+
+ findTable().vm.$emit('delete-message', id);
+ await waitForPromises();
+
+ expect(redirectTo).toHaveBeenCalledWith(`${TEST_HOST}/admin/broadcast_messages?page=1`);
});
});
diff --git a/spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js b/spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js
deleted file mode 100644
index 0b3e2e2a75a78..0000000000000
--- a/spec/frontend/admin/broadcast_messages/components/messages_table_row_spec.js
+++ /dev/null
@@ -1,26 +0,0 @@
-import { shallowMount } from '@vue/test-utils';
-import MessagesTableRow from '~/admin/broadcast_messages/components/messages_table_row.vue';
-import { MOCK_MESSAGE } from '../mock_data';
-
-describe('MessagesTableRow', () => {
- let wrapper;
-
- function createComponent(props = {}) {
- wrapper = shallowMount(MessagesTableRow, {
- propsData: {
- message: MOCK_MESSAGE,
- ...props,
- },
- });
- }
-
- afterEach(() => {
- wrapper.destroy();
- });
-
- it('renders the message ID', () => {
- createComponent();
-
- expect(wrapper.text()).toBe(`${MOCK_MESSAGE.id}`);
- });
-});
diff --git a/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js b/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js
index 0699fa34024e6..349fab03853b7 100644
--- a/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js
+++ b/spec/frontend/admin/broadcast_messages/components/messages_table_spec.js
@@ -1,15 +1,19 @@
-import { shallowMount } from '@vue/test-utils';
+import { mount } from '@vue/test-utils';
import MessagesTable from '~/admin/broadcast_messages/components/messages_table.vue';
-import MessagesTableRow from '~/admin/broadcast_messages/components/messages_table_row.vue';
import { MOCK_MESSAGES } from '../mock_data';
describe('MessagesTable', () => {
let wrapper;
- const findRows = () => wrapper.findAllComponents(MessagesTableRow);
+ const findRows = () => wrapper.findAll('[data-testid="message-row"]');
+ const findTargetRoles = () => wrapper.find('[data-testid="target-roles-th"]');
+ const findDeleteButton = (id) => wrapper.find(`[data-testid="delete-message-${id}"]`);
- function createComponent(props = {}) {
- wrapper = shallowMount(MessagesTable, {
+ function createComponent(props = {}, glFeatures = {}) {
+ wrapper = mount(MessagesTable, {
+ provide: {
+ glFeatures,
+ },
propsData: {
messages: MOCK_MESSAGES,
...props,
@@ -26,4 +30,22 @@ describe('MessagesTable', () => {
expect(findRows()).toHaveLength(MOCK_MESSAGES.length);
});
+
+ it('renders the "Target Roles" column when roleTargetedBroadcastMessages is enabled', () => {
+ createComponent({}, { roleTargetedBroadcastMessages: true });
+ expect(findTargetRoles().exists()).toBe(true);
+ });
+
+ it('does not render the "Target Roles" column when roleTargetedBroadcastMessages is disabled', () => {
+ createComponent();
+ expect(findTargetRoles().exists()).toBe(false);
+ });
+
+ it('emits a delete-message event when a delete button is clicked', () => {
+ const { id } = MOCK_MESSAGES[0];
+ createComponent();
+ findDeleteButton(id).element.click();
+ expect(wrapper.emitted('delete-message')).toHaveLength(1);
+ expect(wrapper.emitted('delete-message')[0]).toEqual([id]);
+ });
});
diff --git a/spec/frontend/admin/broadcast_messages/mock_data.js b/spec/frontend/admin/broadcast_messages/mock_data.js
index f176e43a53540..8dd98c2319d17 100644
--- a/spec/frontend/admin/broadcast_messages/mock_data.js
+++ b/spec/frontend/admin/broadcast_messages/mock_data.js
@@ -1,5 +1,17 @@
-export const MOCK_MESSAGE = {
- id: 100,
-};
+const generateMockMessage = (id) => ({
+ id,
+ delete_path: `/admin/broadcast_messages/${id}.js`,
+ edit_path: `/admin/broadcast_messages/${id}/edit`,
+ starts_at: new Date().toISOString(),
+ ends_at: new Date().toISOString(),
+ preview: '<div>YEET</div>',
+ status: 'Expired',
+ target_path: '*/welcome',
+ target_roles: 'Maintainer, Owner',
+ type: 'Banner',
+});
-export const MOCK_MESSAGES = [MOCK_MESSAGE, { id: 200 }, { id: 300 }];
+export const generateMockMessages = (n) =>
+ [...Array(n).keys()].map((id) => generateMockMessage(id + 1));
+
+export const MOCK_MESSAGES = generateMockMessages(5).map((id) => generateMockMessage(id));
--
GitLab