diff --git a/app/assets/javascripts/admin/broadcast_messages/components/base.vue b/app/assets/javascripts/admin/broadcast_messages/components/base.vue index bc395a836257c05cafaadd85d408ca68942f89ba..b7bafe46327ab3d4d2148ce42c1b2eeb42b712cb 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 7b531b850c617f75a74ed182d5ffcb6ba7bff26f..1408312d3e433df5b850814c023613f136c57130 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 bd45bcc4fc422d520bbf76777b5be4640ceb895c..0000000000000000000000000000000000000000 --- 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 e71495804ee55d2662c1d1ad5314fa9418938953..81952d2033ee0c7c1738a04e7e9d586342d09187 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 4159e4b206fbb4416089b6c5afa9d77eb6b402d9..edd8541469662fdc970fe17c304eca6a015a5929 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 6f00b3d68fe1cc5d1999c4a9d7ee12af222c2b28..7559365e49a9139a83c292883a5bea09c9d29e21 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 a4ac7b42231a7e6c773c34274dbd9a10ea8b22f6..39259597a6c1fa95100079379f03a10edd1b30de 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 65f828ff6b45ec3298863f897818bd8ff63fda28..020e1c1d7c14c3ffcea6384e30e637a088b8d96f 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 0b3e2e2a75a78e48654db6105e4b20e7c3681163..0000000000000000000000000000000000000000 --- 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 0699fa34024e6414e7ff53bf9fd75e8bcba058ba..349fab03853b7828882b746f7dd5fbb226ca6324 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 f176e43a53540cff60577256ad6ca4be9672bcb2..8dd98c2319d17ade6eb67e04a81116adf21eed33 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));