diff options
Diffstat (limited to 'app')
70 files changed, 1073 insertions, 545 deletions
diff --git a/app/assets/javascripts/jobs/components/empty_state.vue b/app/assets/javascripts/jobs/components/empty_state.vue index 04f910b6b80..275ed80146e 100644 --- a/app/assets/javascripts/jobs/components/empty_state.vue +++ b/app/assets/javascripts/jobs/components/empty_state.vue @@ -1,9 +1,11 @@ <script> import { GlLink } from '@gitlab/ui'; +import ManualVariablesForm from './manual_variables_form.vue'; export default { components: { GlLink, + ManualVariablesForm, }, props: { illustrationPath: { @@ -23,6 +25,21 @@ export default { required: false, default: null, }, + playable: { + type: Boolean, + required: true, + default: false, + }, + scheduled: { + type: Boolean, + required: false, + default: false, + }, + variablesSettingsUrl: { + type: String, + required: false, + default: null, + }, action: { type: Object, required: false, @@ -37,28 +54,40 @@ export default { }, }, }, + computed: { + shouldRenderManualVariables() { + return this.playable && !this.scheduled; + }, + }, }; </script> <template> <div class="row empty-state"> <div class="col-12"> - <div :class="illustrationSizeClass" class="svg-content"><img :src="illustrationPath" /></div> + <div :class="illustrationSizeClass" class="svg-content"> + <img :src="illustrationPath" /> + </div> </div> <div class="col-12"> <div class="text-content"> <h4 class="js-job-empty-state-title text-center">{{ title }}</h4> - <p v-if="content" class="js-job-empty-state-content text-center">{{ content }}</p> - + <p v-if="content" class="js-job-empty-state-content">{{ content }}</p> + </div> + <manual-variables-form + v-if="shouldRenderManualVariables" + :action="action" + :variables-settings-url="variablesSettingsUrl" + /> + <div class="text-content"> <div v-if="action" class="text-center"> <gl-link :href="action.path" :data-method="action.method" class="js-job-empty-state-action btn btn-primary" + >{{ action.button_title }}</gl-link > - {{ action.button_title }} - </gl-link> </div> </div> </div> diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index 79fb67d38cd..ef9fb6d08d1 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -45,6 +45,11 @@ export default { required: false, default: null, }, + variablesSettingsUrl: { + type: String, + required: false, + default: null, + }, runnerHelpUrl: { type: String, required: false, @@ -313,6 +318,9 @@ export default { :title="emptyStateTitle" :content="emptyStateIllustration.content" :action="emptyStateAction" + :playable="job.playable" + :scheduled="job.scheduled" + :variables-settings-url="variablesSettingsUrl" /> <!-- EO empty state --> diff --git a/app/assets/javascripts/jobs/components/manual_variables_form.vue b/app/assets/javascripts/jobs/components/manual_variables_form.vue new file mode 100644 index 00000000000..c32a3cac7be --- /dev/null +++ b/app/assets/javascripts/jobs/components/manual_variables_form.vue @@ -0,0 +1,179 @@ +<script> +import _ from 'underscore'; +import { mapActions } from 'vuex'; +import { GlButton } from '@gitlab/ui'; +import { s__, sprintf } from '~/locale'; +import Icon from '~/vue_shared/components/icon.vue'; + +export default { + name: 'ManualVariablesForm', + components: { + GlButton, + Icon, + }, + props: { + action: { + type: Object, + required: false, + default: null, + validator(value) { + return ( + value === null || + (_.has(value, 'path') && _.has(value, 'method') && _.has(value, 'button_title')) + ); + }, + }, + variablesSettingsUrl: { + type: String, + required: true, + default: '', + }, + }, + inputTypes: { + key: 'key', + value: 'value', + }, + i18n: { + keyPlaceholder: s__('CiVariables|Input variable key'), + valuePlaceholder: s__('CiVariables|Input variable value'), + }, + data() { + return { + variables: [], + key: '', + secretValue: '', + }; + }, + computed: { + helpText() { + return sprintf( + s__( + 'CiVariables|Specify variable values to be used in this run. The values specified in %{linkStart}CI/CD settings%{linkEnd} will be used as default', + ), + { + linkStart: `<a href="${this.variablesSettingsUrl}">`, + linkEnd: '</a>', + }, + false, + ); + }, + }, + watch: { + key(newVal) { + this.handleValueChange(newVal, this.$options.inputTypes.key); + }, + secretValue(newVal) { + this.handleValueChange(newVal, this.$options.inputTypes.value); + }, + }, + methods: { + ...mapActions(['triggerManualJob']), + handleValueChange(newValue, type) { + if (newValue !== '') { + this.createNewVariable(type); + this.resetForm(); + } + }, + createNewVariable(type) { + const newVariable = { + key: this.key, + secret_value: this.secretValue, + id: _.uniqueId(), + }; + + this.variables.push(newVariable); + + return this.$nextTick().then(() => { + this.$refs[`${this.$options.inputTypes[type]}-${newVariable.id}`][0].focus(); + }); + }, + resetForm() { + this.key = ''; + this.secretValue = ''; + }, + deleteVariable(id) { + this.variables.splice(this.variables.findIndex(el => el.id === id), 1); + }, + }, +}; +</script> +<template> + <div class="js-manual-vars-form col-12"> + <label>{{ s__('CiVariables|Variables') }}</label> + + <div class="ci-table"> + <div class="gl-responsive-table-row table-row-header pb-0 pt-0 border-0" role="row"> + <div class="table-section section-50" role="rowheader">{{ s__('CiVariables|Key') }}</div> + <div class="table-section section-50" role="rowheader">{{ s__('CiVariables|Value') }}</div> + </div> + + <div v-for="variable in variables" :key="variable.id" class="gl-responsive-table-row"> + <div class="table-section section-50"> + <div class="table-mobile-header" role="rowheader">{{ s__('Pipeline|Key') }}</div> + <div class="table-mobile-content append-right-10"> + <input + :ref="`${$options.inputTypes.key}-${variable.id}`" + v-model="variable.key" + :placeholder="$options.i18n.keyPlaceholder" + class="ci-variable-body-item form-control" + /> + </div> + </div> + + <div class="table-section section-50"> + <div class="table-mobile-header" role="rowheader">{{ s__('Pipeline|Value') }}</div> + <div class="table-mobile-content append-right-10"> + <input + :ref="`${$options.inputTypes.value}-${variable.id}`" + v-model="variable.secret_value" + :placeholder="$options.i18n.valuePlaceholder" + class="ci-variable-body-item form-control" + /> + </div> + </div> + + <div class="table-section section-10"> + <div class="table-mobile-header" role="rowheader"></div> + <div class="table-mobile-content justify-content-end"> + <gl-button class="btn-transparent btn-blank w-25" @click="deleteVariable(variable.id)"> + <icon name="clear" /> + </gl-button> + </div> + </div> + </div> + <div class="gl-responsive-table-row"> + <div class="table-section section-50"> + <div class="table-mobile-header" role="rowheader">{{ s__('Pipeline|Key') }}</div> + <div class="table-mobile-content append-right-10"> + <input + ref="inputKey" + v-model="key" + class="js-input-key form-control" + :placeholder="$options.i18n.keyPlaceholder" + /> + </div> + </div> + + <div class="table-section section-50"> + <div class="table-mobile-header" role="rowheader">{{ s__('Pipeline|Value') }}</div> + <div class="table-mobile-content append-right-10"> + <input + ref="inputSecretValue" + v-model="secretValue" + class="ci-variable-body-item form-control" + :placeholder="$options.i18n.valuePlaceholder" + /> + </div> + </div> + </div> + </div> + <div class="d-flex prepend-top-default justify-content-center"> + <p class="text-muted" v-html="helpText"></p> + </div> + <div class="d-flex justify-content-center"> + <gl-button variant="primary" @click="triggerManualJob(variables)"> + {{ action.button_title }} + </gl-button> + </div> + </div> +</template> diff --git a/app/assets/javascripts/jobs/index.js b/app/assets/javascripts/jobs/index.js index 25132449458..06514fcce1d 100644 --- a/app/assets/javascripts/jobs/index.js +++ b/app/assets/javascripts/jobs/index.js @@ -15,6 +15,7 @@ export default () => { deploymentHelpUrl: element.dataset.deploymentHelpUrl, runnerHelpUrl: element.dataset.runnerHelpUrl, runnerSettingsUrl: element.dataset.runnerSettingsUrl, + variablesSettingsUrl: element.dataset.variablesSettingsUrl, endpoint: element.dataset.endpoint, pagePath: element.dataset.buildOptionsPagePath, logState: element.dataset.buildOptionsLogState, diff --git a/app/assets/javascripts/jobs/store/actions.js b/app/assets/javascripts/jobs/store/actions.js index 12d67a43599..a2daef96a2d 100644 --- a/app/assets/javascripts/jobs/store/actions.js +++ b/app/assets/javascripts/jobs/store/actions.js @@ -209,5 +209,19 @@ export const receiveJobsForStageError = ({ commit }) => { flash(__('An error occurred while fetching the jobs.')); }; +export const triggerManualJob = ({ state }, variables) => { + const parsedVariables = variables.map(variable => { + const copyVar = Object.assign({}, variable); + delete copyVar.id; + return copyVar; + }); + + axios + .post(state.job.status.action.path, { + job_variables_attributes: parsedVariables, + }) + .catch(() => flash(__('An error occurred while triggering the job.'))); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/lib/utils/color_utils.js b/app/assets/javascripts/lib/utils/color_utils.js new file mode 100644 index 00000000000..07fb2915ca7 --- /dev/null +++ b/app/assets/javascripts/lib/utils/color_utils.js @@ -0,0 +1,25 @@ +/** + * Convert hex color to rgb array + * + * @param hex string + * @returns array|null + */ +export const hexToRgb = hex => { + // Expand shorthand form (e.g. "03F") to full form (e.g. "0033FF") + const shorthandRegex = /^#?([a-f\d])([a-f\d])([a-f\d])$/i; + const fullHex = hex.replace(shorthandRegex, (_m, r, g, b) => r + r + g + g + b + b); + + const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(fullHex); + return result + ? [parseInt(result[1], 16), parseInt(result[2], 16), parseInt(result[3], 16)] + : null; +}; + +export const textColorForBackground = backgroundColor => { + const [r, g, b] = hexToRgb(backgroundColor); + + if (r + g + b > 500) { + return '#333333'; + } + return '#FFFFFF'; +}; diff --git a/app/assets/javascripts/pages/admin/broadcast_messages/broadcast_message.js b/app/assets/javascripts/pages/admin/broadcast_messages/broadcast_message.js index 6e00e31b828..7a6a486f551 100644 --- a/app/assets/javascripts/pages/admin/broadcast_messages/broadcast_message.js +++ b/app/assets/javascripts/pages/admin/broadcast_messages/broadcast_message.js @@ -3,26 +3,31 @@ import _ from 'underscore'; import axios from '~/lib/utils/axios_utils'; import flash from '~/flash'; import { __ } from '~/locale'; +import { textColorForBackground } from '~/lib/utils/color_utils'; export default () => { - $('input#broadcast_message_color').on('input', function onMessageColorInput() { + const $broadcastMessageColor = $('input#broadcast_message_color'); + const $broadcastMessagePreview = $('div.broadcast-message-preview'); + $broadcastMessageColor.on('input', function onMessageColorInput() { const previewColor = $(this).val(); - $('div.broadcast-message-preview').css('background-color', previewColor); + $broadcastMessagePreview.css('background-color', previewColor); }); $('input#broadcast_message_font').on('input', function onMessageFontInput() { const previewColor = $(this).val(); - $('div.broadcast-message-preview').css('color', previewColor); + $broadcastMessagePreview.css('color', previewColor); }); - const previewPath = $('textarea#broadcast_message_message').data('previewPath'); + const $broadcastMessage = $('textarea#broadcast_message_message'); + const previewPath = $broadcastMessage.data('previewPath'); + const $jsBroadcastMessagePreview = $('.js-broadcast-message-preview'); - $('textarea#broadcast_message_message').on( + $broadcastMessage.on( 'input', _.debounce(function onMessageInput() { const message = $(this).val(); if (message === '') { - $('.js-broadcast-message-preview').text(__('Your message here')); + $jsBroadcastMessagePreview.text(__('Your message here')); } else { axios .post(previewPath, { @@ -31,10 +36,40 @@ export default () => { }, }) .then(({ data }) => { - $('.js-broadcast-message-preview').html(data.message); + $jsBroadcastMessagePreview.html(data.message); }) .catch(() => flash(__('An error occurred while rendering preview broadcast message'))); } }, 250), ); + + const updateColorPreview = () => { + const selectedBackgroundColor = $broadcastMessageColor.val(); + const contrastTextColor = textColorForBackground(selectedBackgroundColor); + + // save contrastTextColor to hidden input field + $('input.text-font-color').val(contrastTextColor); + + // Updates the preview color with the hex-color input + const selectedColorStyle = { + backgroundColor: selectedBackgroundColor, + color: contrastTextColor, + }; + + $('.label-color-preview').css(selectedColorStyle); + + return $broadcastMessagePreview.css(selectedColorStyle); + }; + + const setSuggestedColor = e => { + const color = $(e.currentTarget).data('color'); + $broadcastMessageColor + .val(color) + // Notify the form, that color has changed + .trigger('input'); + updateColorPreview(); + return e.preventDefault(); + }; + + $(document).on('click', '.suggest-colors a', setSuggestedColor); }; diff --git a/app/assets/javascripts/pdf/index.vue b/app/assets/javascripts/pdf/index.vue index 75feeb1ba04..14640c172cd 100644 --- a/app/assets/javascripts/pdf/index.vue +++ b/app/assets/javascripts/pdf/index.vue @@ -14,7 +14,6 @@ export default { }, data() { return { - loading: false, pages: [], }; }, @@ -41,17 +40,18 @@ export default { cMapPacked: true, }) .then(this.renderPages) - .then(() => this.$emit('pdflabload')) - .catch(error => this.$emit('pdflaberror', error)) - .then(() => { - this.loading = false; + .then(pages => { + this.pages = pages; + this.$emit('pdflabload'); + }) + .catch(error => { + this.$emit('pdflaberror', error); }); }, renderPages(pdf) { const pagePromises = []; - this.loading = true; for (let num = 1; num <= pdf.numPages; num += 1) { - pagePromises.push(pdf.getPage(num).then(p => this.pages.push(p))); + pagePromises.push(pdf.getPage(num)); } return Promise.all(pagePromises); }, @@ -63,8 +63,8 @@ export default { <div v-if="hasPDF" class="pdf-viewer"> <page v-for="(page, index) in pages" + v-if="page" :key="index" - :v-if="!loading" :page="page" :number="index + 1" /> diff --git a/app/assets/javascripts/pdf/page/index.vue b/app/assets/javascripts/pdf/page/index.vue index f16aaca6cd7..d933fdf220a 100644 --- a/app/assets/javascripts/pdf/page/index.vue +++ b/app/assets/javascripts/pdf/page/index.vue @@ -39,7 +39,9 @@ export default { .then(() => { this.rendering = false; }) - .catch(error => this.$emit('pdflaberror', error)); + .catch(error => { + this.$emit('pdflaberror', error); + }); }, }; </script> diff --git a/app/assets/javascripts/performance_bar/components/detailed_metric.vue b/app/assets/javascripts/performance_bar/components/detailed_metric.vue index d5f1cea8356..5bc1d5e0533 100644 --- a/app/assets/javascripts/performance_bar/components/detailed_metric.vue +++ b/app/assets/javascripts/performance_bar/components/detailed_metric.vue @@ -16,11 +16,14 @@ export default { type: String, required: true, }, - header: { + title: { type: String, - required: true, + required: false, + default() { + return this.metric; + }, }, - details: { + header: { type: String, required: true, }, @@ -34,7 +37,7 @@ export default { return this.currentRequest.details[this.metric]; }, detailsList() { - return this.metricDetails[this.details]; + return this.metricDetails.details; }, }, }; @@ -101,6 +104,6 @@ export default { <div slot="footer"></div> </gl-modal> - {{ metric }} + {{ title }} </div> </template> diff --git a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue index 73c9c60765a..769ddb21277 100644 --- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue +++ b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue @@ -34,23 +34,25 @@ export default { }, }, detailedMetrics: [ - { metric: 'pg', header: s__('PerformanceBar|SQL queries'), details: 'queries', keys: ['sql'] }, + { + metric: 'active-record', + title: 'pg', + header: s__('PerformanceBar|SQL queries'), + keys: ['sql'], + }, { metric: 'gitaly', header: s__('PerformanceBar|Gitaly calls'), - details: 'details', keys: ['feature', 'request'], }, { metric: 'rugged', header: s__('PerformanceBar|Rugged calls'), - details: 'details', keys: ['feature', 'args'], }, { metric: 'redis', header: s__('PerformanceBar|Redis calls'), - details: 'details', keys: ['cmd'], }, ], @@ -118,8 +120,8 @@ export default { :key="metric.metric" :current-request="currentRequest" :metric="metric.metric" + :title="metric.title" :header="metric.header" - :details="metric.details" :keys="metric.keys" /> <div v-if="initialRequest" id="peek-view-rblineprof" class="view"> diff --git a/app/assets/javascripts/visual_review_toolbar/components/comment.js b/app/assets/javascripts/visual_review_toolbar/components/comment.js index 04bfb5e9532..20effc1751d 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/comment.js +++ b/app/assets/javascripts/visual_review_toolbar/components/comment.js @@ -1,148 +1,39 @@ -import { BLACK, COMMENT_BOX, MUTED, LOGOUT } from './constants'; -import { clearNote, postError } from './note'; -import { - buttonClearStyles, - selectCommentBox, - selectCommentButton, - selectNote, - selectNoteContainer, -} from './utils'; - -const comment = ` - <div> - <textarea id="${COMMENT_BOX}" name="${COMMENT_BOX}" rows="3" placeholder="Enter your feedback or idea" class="gitlab-input" aria-required="true"></textarea> - <p class="gitlab-metadata-note">Additional metadata will be included: browser, OS, current page, user agent, and viewport dimensions.</p> - </div> - <div class="gitlab-button-wrapper"> - <button class="gitlab-button gitlab-button-secondary" style="${buttonClearStyles}" type="button" id="${LOGOUT}"> Log out </button> - <button class="gitlab-button gitlab-button-success" style="${buttonClearStyles}" type="button" id="gitlab-comment-button"> Send feedback </button> - </div> -`; - -const resetCommentButton = () => { - const commentButton = selectCommentButton(); - - /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ - commentButton.innerText = 'Send feedback'; - commentButton.classList.replace('gitlab-button-secondary', 'gitlab-button-success'); - commentButton.style.opacity = 1; -}; - -const resetCommentBox = () => { - const commentBox = selectCommentBox(); - commentBox.style.pointerEvents = 'auto'; - commentBox.style.color = BLACK; -}; - -const resetCommentText = () => { - const commentBox = selectCommentBox(); - commentBox.value = ''; -}; - -const resetComment = () => { - resetCommentButton(); - resetCommentBox(); - resetCommentText(); -}; - -const confirmAndClear = feedbackInfo => { - const commentButton = selectCommentButton(); - const currentNote = selectNote(); - const noteContainer = selectNoteContainer(); - - /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ - commentButton.innerText = 'Feedback sent'; - noteContainer.style.visibility = 'visible'; - currentNote.insertAdjacentHTML('beforeend', feedbackInfo); - - setTimeout(resetComment, 1000); - setTimeout(clearNote, 6000); -}; - -const setInProgressState = () => { - const commentButton = selectCommentButton(); - const commentBox = selectCommentBox(); - - /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ - commentButton.innerText = 'Sending feedback'; - commentButton.classList.replace('gitlab-button-success', 'gitlab-button-secondary'); - commentButton.style.opacity = 0.5; - commentBox.style.color = MUTED; - commentBox.style.pointerEvents = 'none'; -}; - -const postComment = ({ - href, - platform, - browser, - userAgent, - innerWidth, - innerHeight, - projectId, - projectPath, - mergeRequestId, - mrUrl, - token, -}) => { - // Clear any old errors - clearNote(COMMENT_BOX); - - setInProgressState(); - - const commentText = selectCommentBox().value.trim(); - - if (!commentText) { - /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ - postError('Your comment appears to be empty.', COMMENT_BOX); - resetCommentBox(); - resetCommentButton(); - return; - } - - const detailText = ` - \n -<details> - <summary>Metadata</summary> - Posted from ${href} | ${platform} | ${browser} | ${innerWidth} x ${innerHeight}. - <br /><br /> - <em>User agent: ${userAgent}</em> -</details> +import { nextView } from '../store'; +import { localStorage, COMMENT_BOX, LOGOUT } from '../shared'; +import { clearNote } from './note'; +import { buttonClearStyles } from './utils'; +import { addForm } from './wrapper'; +import { changeSelectedMr, selectedMrNote } from './comment_mr_note'; +import postComment from './comment_post'; +import { saveComment, getSavedComment } from './comment_storage'; + +const comment = state => { + const savedComment = getSavedComment(); + + return ` + <div> + <textarea id="${COMMENT_BOX}" name="${COMMENT_BOX}" rows="3" placeholder="Enter your feedback or idea" class="gitlab-input" aria-required="true">${savedComment}</textarea> + ${selectedMrNote(state)} + <p class="gitlab-metadata-note">Additional metadata will be included: browser, OS, current page, user agent, and viewport dimensions.</p> + </div> + <div class="gitlab-button-wrapper"> + <button class="gitlab-button gitlab-button-success" style="${buttonClearStyles}" type="button" id="gitlab-comment-button"> Send feedback </button> + <button class="gitlab-button gitlab-button-secondary" style="${buttonClearStyles}" type="button" id="${LOGOUT}"> Log out </button> + </div> `; +}; - const url = ` - ${mrUrl}/api/v4/projects/${projectId}/merge_requests/${mergeRequestId}/discussions`; - - const body = `${commentText} ${detailText}`; - - fetch(url, { - method: 'POST', - headers: { - 'PRIVATE-TOKEN': token, - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ body }), - }) - .then(response => { - if (response.ok) { - return response.json(); - } - - throw new Error(`${response.status}: ${response.statusText}`); - }) - .then(data => { - const commentId = data.notes[0].id; - const feedbackLink = `${mrUrl}/${projectPath}/merge_requests/${mergeRequestId}#note_${commentId}`; - const feedbackInfo = `Feedback sent. View at <a class="gitlab-link" href="${feedbackLink}">${projectPath} #${mergeRequestId} (comment ${commentId})</a>`; - confirmAndClear(feedbackInfo); - }) - .catch(err => { - postError( - `Your comment could not be sent. Please try again. Error: ${err.message}`, - COMMENT_BOX, - ); - resetCommentBox(); - resetCommentButton(); - }); +// This function is here becaause it is called only from the comment view +// If we reach a design where we can logout from multiple views, promote this +// to it's own package +const logoutUser = state => { + localStorage.removeItem('token'); + localStorage.removeItem('mergeRequestId'); + state.token = ''; + state.mergeRequestId = ''; + + clearNote(); + addForm(nextView(state, COMMENT_BOX)); }; -export { comment, postComment }; +export { changeSelectedMr, comment, logoutUser, postComment, saveComment }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/comment_mr_note.js b/app/assets/javascripts/visual_review_toolbar/components/comment_mr_note.js new file mode 100644 index 00000000000..f71ffbf4f20 --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/components/comment_mr_note.js @@ -0,0 +1,31 @@ +import { nextView } from '../store'; +import { localStorage, CHANGE_MR_ID_BUTTON, COMMENT_BOX } from '../shared'; +import { clearNote } from './note'; +import { buttonClearStyles } from './utils'; +import { addForm } from './wrapper'; + +const selectedMrNote = state => { + const { mrUrl, projectPath, mergeRequestId } = state; + + const mrLink = `${mrUrl}/${projectPath}/merge_requests/${mergeRequestId}`; + + return ` + <p class="gitlab-metadata-note"> + This posts to merge request <a class="gitlab-link" href="${mrLink}">!${mergeRequestId}</a>. + <button style="${buttonClearStyles}" type="button" id="${CHANGE_MR_ID_BUTTON}" class="gitlab-link gitlab-link-button">Change</button> + </p> + `; +}; + +const clearMrId = state => { + localStorage.removeItem('mergeRequestId'); + state.mergeRequestId = ''; +}; + +const changeSelectedMr = state => { + clearMrId(state); + clearNote(); + addForm(nextView(state, COMMENT_BOX)); +}; + +export { changeSelectedMr, selectedMrNote }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/comment_post.js b/app/assets/javascripts/visual_review_toolbar/components/comment_post.js new file mode 100644 index 00000000000..ee5f2b62425 --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/components/comment_post.js @@ -0,0 +1,145 @@ +import { BLACK, COMMENT_BOX, MUTED } from '../shared'; +import { clearSavedComment } from './comment_storage'; +import { clearNote, postError } from './note'; +import { selectCommentBox, selectCommentButton, selectNote, selectNoteContainer } from './utils'; + +const resetCommentButton = () => { + const commentButton = selectCommentButton(); + + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + commentButton.innerText = 'Send feedback'; + commentButton.classList.replace('gitlab-button-secondary', 'gitlab-button-success'); + commentButton.style.opacity = 1; +}; + +const resetCommentBox = () => { + const commentBox = selectCommentBox(); + commentBox.style.pointerEvents = 'auto'; + commentBox.style.color = BLACK; +}; + +const resetCommentText = () => { + const commentBox = selectCommentBox(); + commentBox.value = ''; + clearSavedComment(); +}; + +const resetComment = () => { + resetCommentButton(); + resetCommentBox(); + resetCommentText(); +}; + +const confirmAndClear = feedbackInfo => { + const commentButton = selectCommentButton(); + const currentNote = selectNote(); + const noteContainer = selectNoteContainer(); + + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + commentButton.innerText = 'Feedback sent'; + noteContainer.style.visibility = 'visible'; + currentNote.insertAdjacentHTML('beforeend', feedbackInfo); + + setTimeout(resetComment, 1000); + setTimeout(clearNote, 6000); +}; + +const setInProgressState = () => { + const commentButton = selectCommentButton(); + const commentBox = selectCommentBox(); + + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + commentButton.innerText = 'Sending feedback'; + commentButton.classList.replace('gitlab-button-success', 'gitlab-button-secondary'); + commentButton.style.opacity = 0.5; + commentBox.style.color = MUTED; + commentBox.style.pointerEvents = 'none'; +}; + +const commentErrors = error => { + switch (error.status) { + case 401: + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + return 'Unauthorized. You may have entered an incorrect authentication token.'; + case 404: + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + return 'Not found. You may have entered an incorrect merge request ID.'; + default: + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + return `Your comment could not be sent. Please try again. Error: ${error.message}`; + } +}; + +const postComment = ({ + platform, + browser, + userAgent, + innerWidth, + innerHeight, + projectId, + projectPath, + mergeRequestId, + mrUrl, + token, +}) => { + // Clear any old errors + clearNote(COMMENT_BOX); + + setInProgressState(); + + const commentText = selectCommentBox().value.trim(); + // Get the href at the last moment to support SPAs + const { href } = window.location; + + if (!commentText) { + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + postError('Your comment appears to be empty.', COMMENT_BOX); + resetCommentBox(); + resetCommentButton(); + return; + } + + const detailText = ` + \n +<details> + <summary>Metadata</summary> + Posted from ${href} | ${platform} | ${browser} | ${innerWidth} x ${innerHeight}. + <br /><br /> + <em>User agent: ${userAgent}</em> +</details> + `; + + const url = ` + ${mrUrl}/api/v4/projects/${projectId}/merge_requests/${mergeRequestId}/discussions`; + + const body = `${commentText} ${detailText}`; + + fetch(url, { + method: 'POST', + headers: { + 'PRIVATE-TOKEN': token, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ body }), + }) + .then(response => { + if (response.ok) { + return response.json(); + } + + throw response; + }) + .then(data => { + const commentId = data.notes[0].id; + const feedbackLink = `${mrUrl}/${projectPath}/merge_requests/${mergeRequestId}#note_${commentId}`; + const feedbackInfo = `Feedback sent. View at <a class="gitlab-link" href="${feedbackLink}">${projectPath} !${mergeRequestId} (comment ${commentId})</a>`; + confirmAndClear(feedbackInfo); + }) + .catch(err => { + postError(commentErrors(err), COMMENT_BOX); + resetCommentBox(); + resetCommentButton(); + }); +}; + +export default postComment; diff --git a/app/assets/javascripts/visual_review_toolbar/components/comment_storage.js b/app/assets/javascripts/visual_review_toolbar/components/comment_storage.js new file mode 100644 index 00000000000..32a9e7e2f05 --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/components/comment_storage.js @@ -0,0 +1,20 @@ +import { selectCommentBox } from './utils'; +import { sessionStorage } from '../shared'; + +const getSavedComment = () => sessionStorage.getItem('comment') || ''; + +const saveComment = () => { + const currentComment = selectCommentBox(); + + // This may be added to any view via top-level beforeunload listener + // so let's skip if it does not apply + if (currentComment && currentComment.value) { + sessionStorage.setItem('comment', currentComment.value); + } +}; + +const clearSavedComment = () => { + sessionStorage.removeItem('comment'); +}; + +export { getSavedComment, saveComment, clearSavedComment }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/form_elements.js b/app/assets/javascripts/visual_review_toolbar/components/form_elements.js new file mode 100644 index 00000000000..608488a6fea --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/components/form_elements.js @@ -0,0 +1,17 @@ +import { REMEMBER_ITEM } from '../shared'; +import { buttonClearStyles } from './utils'; + +/* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ +const rememberBox = (rememberText = 'Remember me') => ` + <div class="gitlab-checkbox-wrapper"> + <input type="checkbox" id="${REMEMBER_ITEM}" name="${REMEMBER_ITEM}" value="remember"> + <label for="${REMEMBER_ITEM}" class="gitlab-checkbox-label">${rememberText}</label> + </div> +`; + +const submitButton = buttonId => ` + <div class="gitlab-button-wrapper"> + <button class="gitlab-button-wide gitlab-button gitlab-button-success" style="${buttonClearStyles}" type="button" id="${buttonId}"> Submit </button> + </div> +`; +export { rememberBox, submitButton }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/index.js b/app/assets/javascripts/visual_review_toolbar/components/index.js index 50b52d7d3a2..e88b3637ad8 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/index.js +++ b/app/assets/javascripts/visual_review_toolbar/components/index.js @@ -1,33 +1,23 @@ -import { comment, postComment } from './comment'; -import { - COLLAPSE_BUTTON, - COMMENT_BUTTON, - FORM_CONTAINER, - LOGIN, - LOGOUT, - REVIEW_CONTAINER, -} from './constants'; +import { changeSelectedMr, comment, logoutUser, postComment, saveComment } from './comment'; import { authorizeUser, login } from './login'; +import { addMr, mrForm } from './mr_id'; import { note } from './note'; -import { selectContainer } from './utils'; -import { buttonAndForm, logoutUser, toggleForm } from './wrapper'; -import { collapseButton } from './wrapper_icons'; +import { selectContainer, selectForm } from './utils'; +import { buttonAndForm, toggleForm } from './wrapper'; export { + addMr, authorizeUser, buttonAndForm, - collapseButton, + changeSelectedMr, comment, login, logoutUser, + mrForm, note, postComment, + saveComment, selectContainer, + selectForm, toggleForm, - COLLAPSE_BUTTON, - COMMENT_BUTTON, - FORM_CONTAINER, - LOGIN, - LOGOUT, - REVIEW_CONTAINER, }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/login.js b/app/assets/javascripts/visual_review_toolbar/components/login.js index 0a71299f041..4a6976ef2fd 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/login.js +++ b/app/assets/javascripts/visual_review_toolbar/components/login.js @@ -1,35 +1,31 @@ -import { LOGIN, REMEMBER_TOKEN, TOKEN_BOX } from './constants'; +import { nextView } from '../store'; +import { localStorage, LOGIN, TOKEN_BOX } from '../shared'; import { clearNote, postError } from './note'; -import { buttonClearStyles, selectRemember, selectToken } from './utils'; -import { addCommentForm } from './wrapper'; +import { rememberBox, submitButton } from './form_elements'; +import { selectRemember, selectToken } from './utils'; +import { addForm } from './wrapper'; + +const labelText = ` + Enter your <a class="gitlab-link" href="https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html">personal access token</a> +`; const login = ` - <div> - <label for="${TOKEN_BOX}" class="gitlab-label">Enter your <a class="gitlab-link" href="https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html">personal access token</a></label> - <input class="gitlab-input" type="password" id="${TOKEN_BOX}" name="${TOKEN_BOX}" aria-required="true" autocomplete="current-password"> - </div> - <div class="gitlab-checkbox-wrapper"> - <input type="checkbox" id="${REMEMBER_TOKEN}" name="${REMEMBER_TOKEN}" value="remember"> - <label for="${REMEMBER_TOKEN}" class="gitlab-checkbox-label">Remember me</label> - </div> - <div class="gitlab-button-wrapper"> - <button class="gitlab-button-wide gitlab-button gitlab-button-success" style="${buttonClearStyles}" type="button" id="${LOGIN}"> Submit </button> - </div> + <div> + <label for="${TOKEN_BOX}" class="gitlab-label">${labelText}</label> + <input class="gitlab-input" type="password" id="${TOKEN_BOX}" name="${TOKEN_BOX}" autocomplete="current-password" aria-required="true"> + </div> + ${rememberBox()} + ${submitButton(LOGIN)} `; const storeToken = (token, state) => { - const { localStorage } = window; const rememberMe = selectRemember().checked; - // All the browsers we support have localStorage, so let's silently fail - // and go on with the rest of the functionality. - try { - if (rememberMe) { - localStorage.setItem('token', token); - } - } finally { - state.token = token; + if (rememberMe) { + localStorage.setItem('token', token); } + + state.token = token; }; const authorizeUser = state => { @@ -45,7 +41,7 @@ const authorizeUser = state => { } storeToken(token, state); - addCommentForm(); + addForm(nextView(state, LOGIN)); }; -export { authorizeUser, login }; +export { authorizeUser, login, storeToken }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/mr_id.js b/app/assets/javascripts/visual_review_toolbar/components/mr_id.js new file mode 100644 index 00000000000..f51e9631dd2 --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/components/mr_id.js @@ -0,0 +1,63 @@ +import { nextView } from '../store'; +import { MR_ID, MR_ID_BUTTON, localStorage } from '../shared'; +import { clearNote, postError } from './note'; +import { rememberBox, submitButton } from './form_elements'; +import { selectForm, selectMrBox, selectRemember } from './utils'; +import { addForm } from './wrapper'; + +/* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ +const mrLabel = `Enter your merge request ID`; +/* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ +const mrRememberText = `Remember this number`; + +const mrForm = ` + <div> + <label for="${MR_ID}" class="gitlab-label">${mrLabel}</label> + <input class="gitlab-input" type="text" pattern="[1-9][0-9]*" id="${MR_ID}" name="${MR_ID}" placeholder="e.g., 321" aria-required="true"> + </div> + ${rememberBox(mrRememberText)} + ${submitButton(MR_ID_BUTTON)} +`; + +const storeMR = (id, state) => { + const rememberMe = selectRemember().checked; + + if (rememberMe) { + localStorage.setItem('mergeRequestId', id); + } + + state.mergeRequestId = id; +}; + +const getFormError = (mrNumber, form) => { + if (!mrNumber) { + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + return 'Please enter your merge request ID number.'; + } + + if (!form.checkValidity()) { + /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ + return 'Please remove any non-number values from the field.'; + } + + return null; +}; + +const addMr = state => { + // Clear any old errors + clearNote(MR_ID); + + const mrNumber = selectMrBox().value; + const form = selectForm(); + const formError = getFormError(mrNumber, form); + + if (formError) { + postError(formError, MR_ID); + return; + } + + storeMR(mrNumber, state); + addForm(nextView(state, MR_ID)); +}; + +export { addMr, mrForm, storeMR }; diff --git a/app/assets/javascripts/visual_review_toolbar/components/note.js b/app/assets/javascripts/visual_review_toolbar/components/note.js index 0150f640aae..9cddcb710f2 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/note.js +++ b/app/assets/javascripts/visual_review_toolbar/components/note.js @@ -1,4 +1,4 @@ -import { NOTE, NOTE_CONTAINER, RED } from './constants'; +import { NOTE, NOTE_CONTAINER, RED } from '../shared'; import { selectById, selectNote, selectNoteContainer } from './utils'; const note = ` diff --git a/app/assets/javascripts/visual_review_toolbar/components/utils.js b/app/assets/javascripts/visual_review_toolbar/components/utils.js index 00f4460925d..4ec9bd4a32a 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/utils.js +++ b/app/assets/javascripts/visual_review_toolbar/components/utils.js @@ -6,12 +6,13 @@ import { COMMENT_BUTTON, FORM, FORM_CONTAINER, + MR_ID, NOTE, NOTE_CONTAINER, - REMEMBER_TOKEN, + REMEMBER_ITEM, REVIEW_CONTAINER, TOKEN_BOX, -} from './constants'; +} from '../shared'; // this style must be applied inline in a handful of components /* eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings */ @@ -27,9 +28,10 @@ const selectCommentButton = () => document.getElementById(COMMENT_BUTTON); const selectContainer = () => document.getElementById(REVIEW_CONTAINER); const selectForm = () => document.getElementById(FORM); const selectFormContainer = () => document.getElementById(FORM_CONTAINER); +const selectMrBox = () => document.getElementById(MR_ID); const selectNote = () => document.getElementById(NOTE); const selectNoteContainer = () => document.getElementById(NOTE_CONTAINER); -const selectRemember = () => document.getElementById(REMEMBER_TOKEN); +const selectRemember = () => document.getElementById(REMEMBER_ITEM); const selectToken = () => document.getElementById(TOKEN_BOX); export { @@ -41,6 +43,7 @@ export { selectCommentButton, selectForm, selectFormContainer, + selectMrBox, selectNote, selectNoteContainer, selectRemember, diff --git a/app/assets/javascripts/visual_review_toolbar/components/wrapper.js b/app/assets/javascripts/visual_review_toolbar/components/wrapper.js index f2eaf1d7916..fdf8ad7c41f 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/wrapper.js +++ b/app/assets/javascripts/visual_review_toolbar/components/wrapper.js @@ -1,55 +1,32 @@ -import { comment } from './comment'; -import { CLEAR, FORM, FORM_CONTAINER, WHITE } from './constants'; -import { login } from './login'; -import { clearNote } from './note'; +import { CLEAR, FORM, FORM_CONTAINER, WHITE } from '../shared'; import { selectCollapseButton, selectForm, selectFormContainer, selectNoteContainer, } from './utils'; -import { commentIcon, compressIcon } from './wrapper_icons'; +import { collapseButton, commentIcon, compressIcon } from './wrapper_icons'; const form = content => ` - <form id="${FORM}"> + <form id="${FORM}" novalidate> ${content} </form> `; -const buttonAndForm = ({ content, toggleButton }) => ` +const buttonAndForm = content => ` <div id="${FORM_CONTAINER}" class="gitlab-form-open"> - ${toggleButton} + ${collapseButton} ${form(content)} </div> `; -const addCommentForm = () => { +const addForm = nextForm => { const formWrapper = selectForm(); - formWrapper.innerHTML = comment; + formWrapper.innerHTML = nextForm; }; -const addLoginForm = () => { - const formWrapper = selectForm(); - formWrapper.innerHTML = login; -}; - -function logoutUser() { - const { localStorage } = window; - - // All the browsers we support have localStorage, so let's silently fail - // and go on with the rest of the functionality. - try { - localStorage.removeItem('token'); - } catch (err) { - return; - } - - clearNote(); - addLoginForm(); -} - function toggleForm() { - const collapseButton = selectCollapseButton(); + const toggleButton = selectCollapseButton(); const currentForm = selectForm(); const formContainer = selectFormContainer(); const noteContainer = selectNoteContainer(); @@ -84,19 +61,19 @@ function toggleForm() { }, }; - const nextState = collapseButton.classList.contains('gitlab-collapse-open') ? CLOSED : OPEN; + const nextState = toggleButton.classList.contains('gitlab-collapse-open') ? CLOSED : OPEN; const currentVals = stateVals[nextState]; formContainer.classList.replace(...currentVals.containerClasses); formContainer.style.backgroundColor = currentVals.backgroundColor; formContainer.classList.toggle('gitlab-form-open'); currentForm.style.display = currentVals.display; - collapseButton.classList.replace(...currentVals.buttonClasses); - collapseButton.innerHTML = currentVals.icon; + toggleButton.classList.replace(...currentVals.buttonClasses); + toggleButton.innerHTML = currentVals.icon; if (noteContainer && noteContainer.innerText.length > 0) { noteContainer.style.display = currentVals.display; } } -export { addCommentForm, addLoginForm, buttonAndForm, logoutUser, toggleForm }; +export { addForm, buttonAndForm, toggleForm }; diff --git a/app/assets/javascripts/visual_review_toolbar/index.js b/app/assets/javascripts/visual_review_toolbar/index.js index f94eb88835a..67b3fadd772 100644 --- a/app/assets/javascripts/visual_review_toolbar/index.js +++ b/app/assets/javascripts/visual_review_toolbar/index.js @@ -1,7 +1,8 @@ import './styles/toolbar.css'; -import { buttonAndForm, note, selectContainer, REVIEW_CONTAINER } from './components'; -import { debounce, eventLookup, getInitialView, initializeState, updateWindowSize } from './store'; +import { buttonAndForm, note, selectForm, selectContainer } from './components'; +import { REVIEW_CONTAINER } from './shared'; +import { eventLookup, getInitialView, initializeGlobalListeners, initializeState } from './store'; /* @@ -20,7 +21,7 @@ import { debounce, eventLookup, getInitialView, initializeState, updateWindowSiz window.addEventListener('load', () => { initializeState(window, document); - const mainContent = buttonAndForm(getInitialView(window)); + const mainContent = buttonAndForm(getInitialView()); const container = document.createElement('div'); container.setAttribute('id', REVIEW_CONTAINER); container.insertAdjacentHTML('beforeend', note); @@ -29,8 +30,22 @@ window.addEventListener('load', () => { document.body.insertBefore(container, document.body.firstChild); selectContainer().addEventListener('click', event => { - eventLookup(event)(); + eventLookup(event.target.id)(); }); - window.addEventListener('resize', debounce(updateWindowSize.bind(null, window), 200)); + selectForm().addEventListener('submit', event => { + // this is important to prevent the form from adding data + // as URL params and inadvertently revealing secrets + event.preventDefault(); + + const id = + event.target.querySelector('.gitlab-button-wrapper') && + event.target.querySelector('.gitlab-button-wrapper').getElementsByTagName('button')[0] && + event.target.querySelector('.gitlab-button-wrapper').getElementsByTagName('button')[0].id; + + // even if this is called with false, it's ok; it will get the default no-op + eventLookup(id)(); + }); + + initializeGlobalListeners(); }); diff --git a/app/assets/javascripts/visual_review_toolbar/components/constants.js b/app/assets/javascripts/visual_review_toolbar/shared/constants.js index 07fcb179d15..a56ea378b14 100644 --- a/app/assets/javascripts/visual_review_toolbar/components/constants.js +++ b/app/assets/javascripts/visual_review_toolbar/shared/constants.js @@ -1,14 +1,17 @@ // component selectors +const CHANGE_MR_ID_BUTTON = 'gitlab-change-mr'; const COLLAPSE_BUTTON = 'gitlab-collapse'; const COMMENT_BOX = 'gitlab-comment'; const COMMENT_BUTTON = 'gitlab-comment-button'; const FORM = 'gitlab-form'; const FORM_CONTAINER = 'gitlab-form-wrapper'; -const LOGIN = 'gitlab-login'; +const LOGIN = 'gitlab-login-button'; const LOGOUT = 'gitlab-logout-button'; +const MR_ID = 'gitlab-submit-mr'; +const MR_ID_BUTTON = 'gitlab-submit-mr-button'; const NOTE = 'gitlab-validation-note'; const NOTE_CONTAINER = 'gitlab-note-wrapper'; -const REMEMBER_TOKEN = 'gitlab-remember_token'; +const REMEMBER_ITEM = 'gitlab-remember-item'; const REVIEW_CONTAINER = 'gitlab-review-container'; const TOKEN_BOX = 'gitlab-token'; @@ -21,6 +24,7 @@ const RED = 'rgba(219, 59, 33, 1)'; const WHITE = 'rgba(250, 250, 250, 1)'; export { + CHANGE_MR_ID_BUTTON, COLLAPSE_BUTTON, COMMENT_BOX, COMMENT_BUTTON, @@ -28,9 +32,11 @@ export { FORM_CONTAINER, LOGIN, LOGOUT, + MR_ID, + MR_ID_BUTTON, NOTE, NOTE_CONTAINER, - REMEMBER_TOKEN, + REMEMBER_ITEM, REVIEW_CONTAINER, TOKEN_BOX, BLACK, diff --git a/app/assets/javascripts/visual_review_toolbar/shared/index.js b/app/assets/javascripts/visual_review_toolbar/shared/index.js new file mode 100644 index 00000000000..751eae74dde --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/shared/index.js @@ -0,0 +1,49 @@ +import { + CHANGE_MR_ID_BUTTON, + COLLAPSE_BUTTON, + COMMENT_BOX, + COMMENT_BUTTON, + FORM, + FORM_CONTAINER, + LOGIN, + LOGOUT, + MR_ID, + MR_ID_BUTTON, + NOTE, + NOTE_CONTAINER, + REMEMBER_ITEM, + REVIEW_CONTAINER, + TOKEN_BOX, + BLACK, + CLEAR, + MUTED, + RED, + WHITE, +} from './constants'; + +import { localStorage, sessionStorage } from './storage_utils'; + +export { + localStorage, + sessionStorage, + CHANGE_MR_ID_BUTTON, + COLLAPSE_BUTTON, + COMMENT_BOX, + COMMENT_BUTTON, + FORM, + FORM_CONTAINER, + LOGIN, + LOGOUT, + MR_ID, + MR_ID_BUTTON, + NOTE, + NOTE_CONTAINER, + REMEMBER_ITEM, + REVIEW_CONTAINER, + TOKEN_BOX, + BLACK, + CLEAR, + MUTED, + RED, + WHITE, +}; diff --git a/app/assets/javascripts/visual_review_toolbar/shared/storage_utils.js b/app/assets/javascripts/visual_review_toolbar/shared/storage_utils.js new file mode 100644 index 00000000000..00456d3536e --- /dev/null +++ b/app/assets/javascripts/visual_review_toolbar/shared/storage_utils.js @@ -0,0 +1,42 @@ +import { setUsingGracefulStorageFlag } from '../store/state'; + +const TEST_KEY = 'gitlab-storage-test'; + +const createStorageStub = () => { + const items = {}; + + return { + getItem(key) { + return items[key]; + }, + setItem(key, value) { + items[key] = value; + }, + removeItem(key) { + delete items[key]; + }, + }; +}; + +const hasStorageSupport = storage => { + // Support test taken from https://stackoverflow.com/a/11214467/1708147 + try { + storage.setItem(TEST_KEY, TEST_KEY); + storage.removeItem(TEST_KEY); + setUsingGracefulStorageFlag(true); + + return true; + } catch (err) { + setUsingGracefulStorageFlag(false); + return false; + } +}; + +const useGracefulStorage = storage => + // If a browser does not support local storage, let's return a graceful implementation. + hasStorageSupport(storage) ? storage : createStorageStub(); + +const localStorage = useGracefulStorage(window.localStorage); +const sessionStorage = useGracefulStorage(window.sessionStorage); + +export { localStorage, sessionStorage }; diff --git a/app/assets/javascripts/visual_review_toolbar/store/events.js b/app/assets/javascripts/visual_review_toolbar/store/events.js index 93996be8473..c9095c77ef1 100644 --- a/app/assets/javascripts/visual_review_toolbar/store/events.js +++ b/app/assets/javascripts/visual_review_toolbar/store/events.js @@ -1,20 +1,37 @@ import { + addMr, authorizeUser, + changeSelectedMr, logoutUser, postComment, + saveComment, toggleForm, +} from '../components'; + +import { + CHANGE_MR_ID_BUTTON, COLLAPSE_BUTTON, COMMENT_BUTTON, LOGIN, LOGOUT, -} from '../components'; + MR_ID_BUTTON, +} from '../shared'; import { state } from './state'; +import debounce from './utils'; const noop = () => {}; -const eventLookup = ({ target: { id } }) => { +// State needs to be bound here to be acted on +// because these are called by click events and +// as such are called with only the `event` object +const eventLookup = id => { switch (id) { + case CHANGE_MR_ID_BUTTON: + return () => { + saveComment(); + changeSelectedMr(state); + }; case COLLAPSE_BUTTON: return toggleForm; case COMMENT_BUTTON: @@ -22,7 +39,12 @@ const eventLookup = ({ target: { id } }) => { case LOGIN: return authorizeUser.bind(null, state); case LOGOUT: - return logoutUser; + return () => { + saveComment(); + logoutUser(state); + }; + case MR_ID_BUTTON: + return addMr.bind(null, state); default: return noop; } @@ -33,4 +55,19 @@ const updateWindowSize = wind => { state.innerHeight = wind.innerHeight; }; -export { eventLookup, updateWindowSize }; +const initializeGlobalListeners = () => { + window.addEventListener('resize', debounce(updateWindowSize.bind(null, window), 200)); + window.addEventListener('beforeunload', event => { + if (state.usingGracefulStorage) { + // if there is no browser storage support, reloading will lose the comment; this way, the user will be warned + // we assign the return value because it is required by Chrome see: https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload#Example, + event.preventDefault(); + /* eslint-disable-next-line no-param-reassign */ + event.returnValue = ''; + } + + saveComment(); + }); +}; + +export { eventLookup, initializeGlobalListeners }; diff --git a/app/assets/javascripts/visual_review_toolbar/store/index.js b/app/assets/javascripts/visual_review_toolbar/store/index.js index 7143588c0bf..07c8dd6f1d2 100644 --- a/app/assets/javascripts/visual_review_toolbar/store/index.js +++ b/app/assets/javascripts/visual_review_toolbar/store/index.js @@ -1,5 +1,11 @@ -import { eventLookup, updateWindowSize } from './events'; -import { getInitialView, initializeState } from './state'; -import debounce from './utils'; +import { eventLookup, initializeGlobalListeners } from './events'; +import { nextView, getInitialView, initializeState, setUsingGracefulStorageFlag } from './state'; -export { debounce, eventLookup, getInitialView, initializeState, updateWindowSize }; +export { + eventLookup, + getInitialView, + initializeGlobalListeners, + initializeState, + nextView, + setUsingGracefulStorageFlag, +}; diff --git a/app/assets/javascripts/visual_review_toolbar/store/state.js b/app/assets/javascripts/visual_review_toolbar/store/state.js index 22702d524b8..741a5c7d99c 100644 --- a/app/assets/javascripts/visual_review_toolbar/store/state.js +++ b/app/assets/javascripts/visual_review_toolbar/store/state.js @@ -1,8 +1,9 @@ -import { comment, login, collapseButton } from '../components'; +import { comment, login, mrForm } from '../components'; +import { localStorage, COMMENT_BOX, LOGIN, MR_ID } from '../shared'; const state = { browser: '', - href: '', + usingGracefulStorage: '', innerWidth: '', innerHeight: '', mergeRequestId: '', @@ -23,11 +24,31 @@ const getBrowserId = sUsrAg => { return aKeys[nIdx]; }; +const nextView = (appState, form = 'none') => { + const formsList = { + [COMMENT_BOX]: currentState => (currentState.token ? mrForm : login), + [LOGIN]: currentState => (currentState.mergeRequestId ? comment(currentState) : mrForm), + [MR_ID]: currentState => (currentState.token ? comment(currentState) : login), + none: currentState => { + if (!currentState.token) { + return login; + } + + if (currentState.token && !currentState.mergeRequestId) { + return mrForm; + } + + return comment(currentState); + }, + }; + + return formsList[form](appState); +}; + const initializeState = (wind, doc) => { const { innerWidth, innerHeight, - location: { href }, navigator: { platform, userAgent }, } = wind; @@ -39,7 +60,6 @@ const initializeState = (wind, doc) => { // This mutates our default state object above. It's weird but it makes the linter happy. Object.assign(state, { browser, - href, innerWidth, innerHeight, mergeRequestId, @@ -49,30 +69,27 @@ const initializeState = (wind, doc) => { projectPath, userAgent, }); -}; -function getInitialView({ localStorage }) { - const loginView = { - content: login, - toggleButton: collapseButton, - }; + return state; +}; - const commentView = { - content: comment, - toggleButton: collapseButton, - }; +const getInitialView = () => { + const token = localStorage.getItem('token'); + const mrId = localStorage.getItem('mergeRequestId'); - try { - const token = localStorage.getItem('token'); + if (token) { + state.token = token; + } - if (token) { - state.token = token; - return commentView; - } - return loginView; - } catch (err) { - return loginView; + if (mrId) { + state.mergeRequestId = mrId; } -} -export { initializeState, getInitialView, state }; + return nextView(state); +}; + +const setUsingGracefulStorageFlag = flag => { + state.usingGracefulStorage = !flag; +}; + +export { initializeState, getInitialView, nextView, setUsingGracefulStorageFlag, state }; diff --git a/app/assets/javascripts/visual_review_toolbar/styles/toolbar.css b/app/assets/javascripts/visual_review_toolbar/styles/toolbar.css index 6a7b2f52549..e5732fd5d93 100644 --- a/app/assets/javascripts/visual_review_toolbar/styles/toolbar.css +++ b/app/assets/javascripts/visual_review_toolbar/styles/toolbar.css @@ -107,10 +107,14 @@ } .gitlab-button-wrapper { - margin-top: 1rem; + margin-top: 0.5rem; display: flex; align-items: baseline; - justify-content: flex-end; + /* + this makes sure the hit enter to submit picks the correct button + on the comment view + */ + flex-direction: row-reverse; } .gitlab-collapse { @@ -155,6 +159,12 @@ text-decoration: underline; } +.gitlab-link-button { + border: none; + cursor: pointer; + padding: 0 .15rem; +} + .gitlab-message { padding: .25rem 0; margin: 0; @@ -165,7 +175,7 @@ font-size: .7rem; line-height: 1rem; color: #666; - margin-bottom: 0; + margin-bottom: .5rem; } .gitlab-input { diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 94d1e8248fc..6fa2f75be33 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -98,12 +98,13 @@ module IssuableActions render json: { notice: "#{quantity} #{resource_name.pluralize(quantity)} updated" } end - # rubocop:disable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord def discussions - notes = NotesFinder.new(project, current_user, finder_params_for_issuable).execute - .inc_relations_for_view - .includes(:noteable) - .fresh + notes = issuable.discussion_notes + .inc_relations_for_view + .with_notes_filter(notes_filter) + .includes(:noteable) + .fresh if notes_filter != UserPreference::NOTES_FILTERS[:only_comments] notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes) @@ -116,7 +117,7 @@ module IssuableActions render json: discussion_serializer.represent(discussions, context: self) end - # rubocop:enable CodeReuse/ActiveRecord + # rubocop: enable CodeReuse/ActiveRecord private @@ -221,13 +222,4 @@ module IssuableActions def parent @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables end - - # rubocop:disable Gitlab/ModuleWithInstanceVariables - def finder_params_for_issuable - { - target: @issuable, - notes_filter: notes_filter - } - end - # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 59f6d3452a3..f5d35379e10 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -90,7 +90,7 @@ module UploadsActions return unless uploader = build_uploader upload_paths = uploader.upload_paths(params[:filename]) - upload = Upload.find_by(uploader: uploader_class.to_s, path: upload_paths) + upload = Upload.find_by(model: model, uploader: uploader_class.to_s, path: upload_paths) upload&.build_uploader end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/controllers/projects/badges_controller.rb b/app/controllers/projects/badges_controller.rb index 09a384e89ab..66b51b17790 100644 --- a/app/controllers/projects/badges_controller.rb +++ b/app/controllers/projects/badges_controller.rb @@ -3,7 +3,8 @@ class Projects::BadgesController < Projects::ApplicationController layout 'project_settings' before_action :authorize_admin_project!, only: [:index] - before_action :no_cache_headers, except: [:index] + before_action :no_cache_headers, only: [:pipeline, :coverage] + before_action :authorize_read_build!, only: [:pipeline, :coverage] def pipeline pipeline_status = Gitlab::Badge::Pipeline::Status diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 02ff6e872c9..adbc0159358 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -94,7 +94,7 @@ class Projects::JobsController < Projects::ApplicationController def play return respond_422 unless @build.playable? - build = @build.play(current_user) + build = @build.play(current_user, play_params[:job_variables_attributes]) redirect_to build_path(build) end @@ -190,6 +190,10 @@ class Projects::JobsController < Projects::ApplicationController { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } end + def play_params + params.permit(job_variables_attributes: %i[key secret_value]) + end + def trace_artifact_file @trace_artifact_file ||= build.job_artifacts_trace&.file end diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index dcc272aecff..006731c0e66 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -45,7 +45,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont def set_pipeline_variables @pipelines = - if can?(current_user, :read_pipeline, @project) + if can?(current_user, :read_pipeline, @merge_request.source_project) @merge_request.all_pipelines else Ci::Pipeline.none diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2aa2508be16..f4d381244d9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -82,7 +82,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def pipelines - @pipelines = @merge_request.all_pipelines.page(params[:page]).per(30) + set_pipeline_variables + @pipelines = @pipelines.page(params[:page]).per(30) Gitlab::PollingInterval.set_header(response, interval: 10_000) diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb index 284e119ca06..7159d0243a3 100644 --- a/app/controllers/projects/triggers_controller.rb +++ b/app/controllers/projects/triggers_controller.rb @@ -4,7 +4,7 @@ class Projects::TriggersController < Projects::ApplicationController before_action :authorize_admin_build! before_action :authorize_manage_trigger!, except: [:index, :create] before_action :authorize_admin_trigger!, only: [:edit, :update] - before_action :trigger, only: [:take_ownership, :edit, :update, :destroy] + before_action :trigger, only: [:edit, :update, :destroy] layout 'project_settings' @@ -24,16 +24,6 @@ class Projects::TriggersController < Projects::ApplicationController redirect_to project_settings_ci_cd_path(@project, anchor: 'js-pipeline-triggers') end - def take_ownership - if trigger.update(owner: current_user) - flash[:notice] = _('Trigger was re-assigned.') - else - flash[:alert] = _('You could not take ownership of trigger.') - end - - redirect_to project_settings_ci_cd_path(@project, anchor: 'js-pipeline-triggers') - end - def edit end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 37ffd28bf9e..d4ff72c2314 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -284,6 +284,18 @@ class ProjectsController < Projects::ApplicationController end # rubocop: enable CodeReuse/ActiveRecord + def resolve + @project = Project.find(params[:id]) + + if can?(current_user, :read_project, @project) + redirect_to @project + else + render_404 + end + end + + private + # Render project landing depending of which features are available # So if page is not available in the list it renders the next page # @@ -453,14 +465,4 @@ class ProjectsController < Projects::ApplicationController def present_project @project = @project.present(current_user: current_user) end - - def resolve - @project = Project.find(params[:id]) - - if can?(current_user, :read_project, @project) - redirect_to @project - else - render_404 - end - end end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 8c674be58c5..13741548687 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -31,6 +31,8 @@ class SearchController < ApplicationController render_commits if @scope == 'commits' eager_load_user_status if @scope == 'users' + increment_navbar_searches_counter + check_single_commit_result end @@ -70,4 +72,10 @@ class SearchController < ApplicationController redirect_to project_commit_path(@project, only_commit) if found_by_commit_sha end end + + def increment_navbar_searches_counter + return if params[:nav_source] != 'navbar' + + Gitlab::UsageDataCounters::SearchCounter.increment_navbar_searches_count + end end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index f4fbeacfaba..1773ac2d508 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -320,7 +320,6 @@ class IssuableFinder def use_cte_for_search? strong_memoize(:use_cte_for_search) do next false unless search - next false unless Gitlab::Database.postgresql? # Only simple unsorted & simple sorts can use CTE next false if params[:sort].present? && !params[:sort].in?(klass.simple_sorts.keys) diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index 917de249104..f730b015c0a 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -59,35 +59,16 @@ class MembersFinder def distinct_on(union) # We're interested in a list of members without duplicates by user_id. # We prefer project members over group members, project members should go first. - if Gitlab::Database.postgresql? - <<~SQL - SELECT DISTINCT ON (user_id, invite_email) member_union.* - FROM (#{union.to_sql}) AS member_union - ORDER BY user_id, - invite_email, - CASE - WHEN type = 'ProjectMember' THEN 1 - WHEN type = 'GroupMember' THEN 2 - ELSE 3 - END - SQL - else - # Older versions of MySQL do not support window functions (and DISTINCT ON is postgres-specific). - <<~SQL - SELECT t1.* - FROM (#{union.to_sql}) AS t1 - JOIN ( - SELECT - COALESCE(user_id, -1) AS user_id, - COALESCE(invite_email, 'NULL') AS invite_email, - MIN(CASE WHEN type = 'ProjectMember' THEN 1 WHEN type = 'GroupMember' THEN 2 ELSE 3 END) AS type_number - FROM - (#{union.to_sql}) AS t3 - GROUP BY COALESCE(user_id, -1), COALESCE(invite_email, 'NULL') - ) AS t2 ON COALESCE(t1.user_id, -1) = t2.user_id - AND COALESCE(t1.invite_email, 'NULL') = t2.invite_email - AND CASE WHEN t1.type = 'ProjectMember' THEN 1 WHEN t1.type = 'GroupMember' THEN 2 ELSE 3 END = t2.type_number - SQL - end + <<~SQL + SELECT DISTINCT ON (user_id, invite_email) member_union.* + FROM (#{union.to_sql}) AS member_union + ORDER BY user_id, + invite_email, + CASE + WHEN type = 'ProjectMember' THEN 1 + WHEN type = 'GroupMember' THEN 2 + ELSE 3 + END + SQL end end diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index c25600221d7..8f610d7dddb 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -3,8 +3,6 @@ class NotesFinder FETCH_OVERLAP = 5.seconds - attr_reader :target_type - # Used to filter Notes # When used with target_type and target_id this returns notes specifically for the controller # @@ -12,7 +10,6 @@ class NotesFinder # current_user - which user check authorizations with # project - which project to look for notes on # params: - # target: noteable # target_type: string # target_id: integer # last_fetched_at: time @@ -21,8 +18,7 @@ class NotesFinder def initialize(project, current_user, params = {}) @project = project @current_user = current_user - @params = params.dup - @target_type = @params[:target_type] + @params = params end def execute @@ -36,27 +32,7 @@ class NotesFinder def target return @target if defined?(@target) - if target_given? - use_explicit_target - else - find_target_by_type_and_ids - end - end - - private - - def target_given? - @params.key?(:target) - end - - def use_explicit_target - @target = @params[:target] - @target_type = @target.class.name.underscore - - @target - end - - def find_target_by_type_and_ids + target_type = @params[:target_type] target_id = @params[:target_id] target_iid = @params[:target_iid] @@ -69,11 +45,13 @@ class NotesFinder @project.commit(target_id) end else - noteable_for_type_by_id(target_type, target_id, target_iid) + noteables_for_type_by_id(target_type, target_id, target_iid) end end - def noteable_for_type_by_id(type, id, iid) + private + + def noteables_for_type_by_id(type, id, iid) query = if id { id: id } else @@ -99,6 +77,10 @@ class NotesFinder search(notes) end + def target_type + @params[:target_type] + end + # rubocop: disable CodeReuse/ActiveRecord def notes_of_any_type types = %w(commit issue merge_request snippet) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index da70cb9a9a7..07813e03f3a 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -40,6 +40,7 @@ module Ci has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent + has_many :job_variables, class_name: 'Ci::JobVariable', foreign_key: :job_id Ci::JobArtifact.file_types.each do |key, value| has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id @@ -48,6 +49,7 @@ module Ci has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build accepts_nested_attributes_for :runner_session + accepts_nested_attributes_for :job_variables delegate :url, to: :runner_session, prefix: true, allow_nil: true delegate :terminal_specification, to: :runner_session, allow_nil: true @@ -331,10 +333,10 @@ module Ci end # rubocop: disable CodeReuse/ServiceClass - def play(current_user) + def play(current_user, job_variables_attributes = nil) Ci::PlayBuildService .new(project, current_user) - .execute(self) + .execute(self, job_variables_attributes) end # rubocop: enable CodeReuse/ServiceClass @@ -432,6 +434,7 @@ module Ci Gitlab::Ci::Variables::Collection.new .concat(persisted_variables) .concat(scoped_variables) + .concat(job_variables) .concat(persisted_environment_variables) .to_runner_variables end diff --git a/app/models/ci/job_variable.rb b/app/models/ci/job_variable.rb new file mode 100644 index 00000000000..862a0bc1299 --- /dev/null +++ b/app/models/ci/job_variable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Ci + class JobVariable < ApplicationRecord + extend Gitlab::Ci::Model + include NewHasVariable + + belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id + + alias_attribute :secret_value, :value + + validates :key, uniqueness: { scope: :job_id } + end +end diff --git a/app/models/concerns/new_has_variable.rb b/app/models/concerns/new_has_variable.rb new file mode 100644 index 00000000000..429bf496872 --- /dev/null +++ b/app/models/concerns/new_has_variable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module NewHasVariable + extend ActiveSupport::Concern + include HasVariable + + included do + attr_encrypted :value, + mode: :per_attribute_iv, + algorithm: 'aes-256-gcm', + key: Settings.attr_encrypted_db_key_base_32, + insecure_mode: false + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 68e6e48fb7d..8ade91933a4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1249,15 +1249,8 @@ class MergeRequest < ApplicationRecord end def all_commits - # MySQL doesn't support LIMIT in a subquery. - diffs_relation = if Gitlab::Database.postgresql? - merge_request_diffs.recent - else - merge_request_diffs - end - MergeRequestDiffCommit - .where(merge_request_diff: diffs_relation) + .where(merge_request_diff: merge_request_diffs.recent) .limit(10_000) end diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 37c129e843a..2ad2838111e 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -149,29 +149,10 @@ class Milestone < ApplicationRecord end def self.upcoming_ids(projects, groups) - rel = unscoped - .for_projects_and_groups(projects, groups) - .active.where('milestones.due_date > CURRENT_DATE') - - if Gitlab::Database.postgresql? - rel.order(:project_id, :group_id, :due_date).select('DISTINCT ON (project_id, group_id) id') - else - # We need to use MySQL's NULL-safe comparison operator `<=>` here - # because one of `project_id` or `group_id` is always NULL - join_clause = <<~HEREDOC - LEFT OUTER JOIN milestones earlier_milestones - ON milestones.project_id <=> earlier_milestones.project_id - AND milestones.group_id <=> earlier_milestones.group_id - AND milestones.due_date > earlier_milestones.due_date - AND earlier_milestones.due_date > CURRENT_DATE - AND earlier_milestones.state = 'active' - HEREDOC - - rel - .joins(join_clause) - .where('earlier_milestones.id IS NULL') - .select(:id) - end + unscoped + .for_projects_and_groups(projects, groups) + .active.where('milestones.due_date > CURRENT_DATE') + .order(:project_id, :group_id, :due_date).select('DISTINCT ON (project_id, group_id) id') end def participants diff --git a/app/models/redirect_route.rb b/app/models/redirect_route.rb index 2e4769364c6..22f60802257 100644 --- a/app/models/redirect_route.rb +++ b/app/models/redirect_route.rb @@ -11,11 +11,7 @@ class RedirectRoute < ApplicationRecord uniqueness: { case_sensitive: false } scope :matching_path_and_descendants, -> (path) do - wheres = if Gitlab::Database.postgresql? - 'LOWER(redirect_routes.path) = LOWER(?) OR LOWER(redirect_routes.path) LIKE LOWER(?)' - else - 'redirect_routes.path = ? OR redirect_routes.path LIKE ?' - end + wheres = 'LOWER(redirect_routes.path) = LOWER(?) OR LOWER(redirect_routes.path) LIKE LOWER(?)' where(wheres, path, "#{sanitize_sql_like(path)}/%") end diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb index 36e601f45c5..82139855760 100644 --- a/app/serializers/issue_entity.rb +++ b/app/serializers/issue_entity.rb @@ -16,9 +16,14 @@ class IssueEntity < IssuableEntity expose :discussion_locked expose :assignees, using: API::Entities::UserBasic expose :due_date - expose :moved_to_id expose :project_id + expose :moved_to_id do |issue| + if issue.moved_to_id.present? && can?(request.current_user, :read_issue, issue.moved_to) + issue.moved_to_id + end + end + expose :web_url do |issue| project_issue_path(issue.project, issue) end diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb index eb0b070657d..9f922ffde81 100644 --- a/app/services/ci/play_build_service.rb +++ b/app/services/ci/play_build_service.rb @@ -2,7 +2,7 @@ module Ci class PlayBuildService < ::BaseService - def execute(build) + def execute(build, job_variables_attributes = nil) unless can?(current_user, :update_build, build) raise Gitlab::Access::AccessDeniedError end @@ -10,7 +10,7 @@ module Ci # Try to enqueue the build, otherwise create a duplicate. # if build.enqueue - build.tap { |action| action.update(user: current_user) } + build.tap { |action| action.update(user: current_user, job_variables_attributes: job_variables_attributes || []) } else Ci::Build.retry(build, current_user) end diff --git a/app/services/clusters/refresh_service.rb b/app/services/clusters/refresh_service.rb deleted file mode 100644 index 3752a306793..00000000000 --- a/app/services/clusters/refresh_service.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -module Clusters - class RefreshService - def self.create_or_update_namespaces_for_cluster(cluster) - projects_with_missing_kubernetes_namespaces_for_cluster(cluster).each do |project| - create_or_update_namespace(cluster, project) - end - end - - def self.create_or_update_namespaces_for_project(project) - clusters_with_missing_kubernetes_namespaces_for_project(project).each do |cluster| - create_or_update_namespace(cluster, project) - end - end - - def self.projects_with_missing_kubernetes_namespaces_for_cluster(cluster) - cluster.all_projects.missing_kubernetes_namespace(cluster.kubernetes_namespaces) - end - - private_class_method :projects_with_missing_kubernetes_namespaces_for_cluster - - def self.clusters_with_missing_kubernetes_namespaces_for_project(project) - project.clusters.managed.missing_kubernetes_namespace(project.kubernetes_namespaces) - end - - private_class_method :clusters_with_missing_kubernetes_namespaces_for_project - - def self.create_or_update_namespace(cluster, project) - kubernetes_namespace = cluster.find_or_initialize_kubernetes_namespace_for_project(project) - - ::Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService.new( - cluster: cluster, - kubernetes_namespace: kubernetes_namespace - ).execute - end - - private_class_method :create_or_update_namespace - end -end diff --git a/app/services/cohorts_service.rb b/app/services/cohorts_service.rb index 6d466c2fc9c..97fbb70f350 100644 --- a/app/services/cohorts_service.rb +++ b/app/services/cohorts_service.rb @@ -95,10 +95,6 @@ class CohortsService # rubocop: enable CodeReuse/ActiveRecord def column_to_date(column) - if Gitlab::Database.postgresql? - "CAST(DATE_TRUNC('month', #{column}) AS date)" - else - "STR_TO_DATE(DATE_FORMAT(#{column}, '%Y-%m-01'), '%Y-%m-%d')" - end + "CAST(DATE_TRUNC('month', #{column}) AS date)" end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 109c964e577..b28f80939ae 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -11,15 +11,18 @@ module MergeRequests # https://gitlab.com/gitlab-org/gitlab-ce/issues/53658 merge_quick_actions_into_params!(merge_request, only: [:target_branch]) merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch) - merge_request.assign_attributes(params) + # Assign the projects first so we can use policies for `filter_params` merge_request.author = current_user + merge_request.source_project = find_source_project + merge_request.target_project = find_target_project + + filter_params(merge_request) + merge_request.assign_attributes(params.to_h.compact) + merge_request.compare_commits = [] - merge_request.source_project = find_source_project - merge_request.target_project = find_target_project - merge_request.target_branch = find_target_branch - merge_request.can_be_created = projects_and_branches_valid? - ensure_milestone_available(merge_request) + merge_request.target_branch = find_target_branch + merge_request.can_be_created = projects_and_branches_valid? # compare branches only if branches are valid, otherwise # compare_branches may raise an error @@ -50,12 +53,14 @@ module MergeRequests to: :merge_request def find_source_project + source_project = project_from_params(:source_project) return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project) project end def find_target_project + target_project = project_from_params(:target_project) return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project) target_project = project.default_merge_request_target @@ -65,6 +70,17 @@ module MergeRequests project end + def project_from_params(param_name) + project_from_params = params.delete(param_name) + + id_param_name = :"#{param_name}_id" + if project_from_params.nil? && params[id_param_name] + project_from_params = Project.find_by_id(params.delete(id_param_name)) + end + + project_from_params + end + def find_target_branch target_branch || target_project.default_branch end diff --git a/app/services/projects/base_move_relations_service.rb b/app/services/projects/base_move_relations_service.rb index 24dec1f3a45..3a159cef58b 100644 --- a/app/services/projects/base_move_relations_service.rb +++ b/app/services/projects/base_move_relations_service.rb @@ -10,17 +10,5 @@ module Projects true end - - private - - # rubocop: disable CodeReuse/ActiveRecord - def prepare_relation(relation, id_param = :id) - if Gitlab::Database.postgresql? - relation - else - relation.model.where("#{id_param}": relation.pluck(id_param)) - end - end - # rubocop: enable CodeReuse/ActiveRecord end end diff --git a/app/services/projects/fetch_statistics_increment_service.rb b/app/services/projects/fetch_statistics_increment_service.rb index 8644e6bf313..b150fd2d9f1 100644 --- a/app/services/projects/fetch_statistics_increment_service.rb +++ b/app/services/projects/fetch_statistics_increment_service.rb @@ -12,14 +12,9 @@ module Projects increment_fetch_count_sql = <<~SQL INSERT INTO #{table_name} (project_id, date, fetch_count) VALUES (#{project.id}, '#{Date.today}', 1) + ON CONFLICT (project_id, date) DO UPDATE SET fetch_count = #{table_name}.fetch_count + 1 SQL - increment_fetch_count_sql += if Gitlab::Database.postgresql? - "ON CONFLICT (project_id, date) DO UPDATE SET fetch_count = #{table_name}.fetch_count + 1" - else - "ON DUPLICATE KEY UPDATE fetch_count = #{table_name}.fetch_count + 1" - end - ActiveRecord::Base.connection.execute(increment_fetch_count_sql) end diff --git a/app/services/projects/move_deploy_keys_projects_service.rb b/app/services/projects/move_deploy_keys_projects_service.rb index b6a3af8c7b8..01419563538 100644 --- a/app/services/projects/move_deploy_keys_projects_service.rb +++ b/app/services/projects/move_deploy_keys_projects_service.rb @@ -16,8 +16,7 @@ module Projects private def move_deploy_keys_projects - prepare_relation(non_existent_deploy_keys_projects) - .update_all(project_id: @project.id) + non_existent_deploy_keys_projects.update_all(project_id: @project.id) end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/services/projects/move_lfs_objects_projects_service.rb b/app/services/projects/move_lfs_objects_projects_service.rb index 308a54ad06e..10e19014db4 100644 --- a/app/services/projects/move_lfs_objects_projects_service.rb +++ b/app/services/projects/move_lfs_objects_projects_service.rb @@ -16,8 +16,7 @@ module Projects private def move_lfs_objects_projects - prepare_relation(non_existent_lfs_objects_projects) - .update_all(project_id: @project.lfs_storage_project.id) + non_existent_lfs_objects_projects.update_all(project_id: @project.lfs_storage_project.id) end def remove_remaining_lfs_objects_project diff --git a/app/services/projects/move_notification_settings_service.rb b/app/services/projects/move_notification_settings_service.rb index e740c44bd26..65a888fe26b 100644 --- a/app/services/projects/move_notification_settings_service.rb +++ b/app/services/projects/move_notification_settings_service.rb @@ -16,8 +16,7 @@ module Projects private def move_notification_settings - prepare_relation(non_existent_notifications) - .update_all(source_id: @project.id) + non_existent_notifications.update_all(source_id: @project.id) end # Remove remaining notification settings from source_project diff --git a/app/services/projects/move_project_authorizations_service.rb b/app/services/projects/move_project_authorizations_service.rb index 2985ba89014..c95ad60ab5e 100644 --- a/app/services/projects/move_project_authorizations_service.rb +++ b/app/services/projects/move_project_authorizations_service.rb @@ -21,8 +21,7 @@ module Projects private def move_project_authorizations - prepare_relation(non_existent_authorization, :user_id) - .update_all(project_id: @project.id) + non_existent_authorization.update_all(project_id: @project.id) end def remove_remaining_authorizations diff --git a/app/services/projects/move_project_group_links_service.rb b/app/services/projects/move_project_group_links_service.rb index cf4b291c761..d1aa9af2bcb 100644 --- a/app/services/projects/move_project_group_links_service.rb +++ b/app/services/projects/move_project_group_links_service.rb @@ -20,8 +20,7 @@ module Projects private def move_group_links - prepare_relation(non_existent_group_links) - .update_all(project_id: @project.id) + non_existent_group_links.update_all(project_id: @project.id) end # Remove remaining project group links from source_project diff --git a/app/services/projects/move_project_members_service.rb b/app/services/projects/move_project_members_service.rb index faf389241d2..de4e7e5a1e3 100644 --- a/app/services/projects/move_project_members_service.rb +++ b/app/services/projects/move_project_members_service.rb @@ -20,7 +20,7 @@ module Projects private def move_project_members - prepare_relation(non_existent_members).update_all(source_id: @project.id) + non_existent_members.update_all(source_id: @project.id) end def remove_remaining_members diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb index 3b2a9d2f80e..967fcdc704e 100644 --- a/app/uploaders/records_uploads.rb +++ b/app/uploaders/records_uploads.rb @@ -27,7 +27,7 @@ module RecordsUploads end def readd_upload - uploads.where(path: upload_path).delete_all + uploads.where(model: model, path: upload_path).delete_all upload.delete if upload self.upload = build_upload.tap(&:save!) diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml index c8ee87c6212..962234d3aea 100644 --- a/app/views/admin/broadcast_messages/_form.html.haml +++ b/app/views/admin/broadcast_messages/_form.html.haml @@ -17,19 +17,27 @@ required: true, dir: 'auto', data: { preview_path: preview_admin_broadcast_messages_path } - .form-group.row.js-toggle-colors-container - .col-sm-10.offset-sm-2 - = link_to 'Customize colors', '#', class: 'js-toggle-colors-link' - .form-group.row.js-toggle-colors-container.toggle-colors.hide + .form-group.row .col-sm-2.col-form-label - = f.label :color, "Background Color" + = f.label :color, _("Background color") .col-sm-10 - = f.color_field :color, class: "form-control" + .input-group + .input-group-prepend + .input-group-text.label-color-preview{ :style => 'background-color: ' + @broadcast_message.color + '; color: ' + @broadcast_message.font } + = ' '.html_safe + = f.text_field :color, class: "form-control" + .form-text.text-muted + = _('Choose any color.') + %br + = _("Or you can choose one of the suggested colors below") + + = render_suggested_colors + .form-group.row.js-toggle-colors-container.toggle-colors.hide .col-sm-2.col-form-label = f.label :font, "Font Color" .col-sm-10 - = f.color_field :font, class: "form-control" + = f.color_field :font, class: "form-control text-font-color" .form-group.row .col-sm-2.col-form-label = f.label :starts_at, _("Starts at (UTC)") diff --git a/app/views/layouts/_search.html.haml b/app/views/layouts/_search.html.haml index c62dce880c0..9cac8266eaf 100644 --- a/app/views/layouts/_search.html.haml +++ b/app/views/layouts/_search.html.haml @@ -45,5 +45,6 @@ - if @snippet || @snippets = hidden_field_tag :snippets, true = hidden_field_tag :repository_ref, @ref + = hidden_field_tag :nav_source, 'navbar' = button_tag 'Go' if ENV['RAILS_ENV'] == 'test' .search-autocomplete-opts.hide{ :'data-autocomplete-path' => search_autocomplete_path, :'data-autocomplete-project-id' => @project.try(:id), :'data-autocomplete-project-ref' => @ref } diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index 81a53f22f67..c7fab87a593 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -11,4 +11,5 @@ deployment_help_url: help_page_path('user/project/clusters/index.html', anchor: 'troubleshooting-failed-deployment-jobs'), runner_help_url: help_page_path('ci/runners/README.html', anchor: 'setting-maximum-job-timeout-for-a-runner'), runner_settings_url: project_runners_path(@build.project, anchor: 'js-runners-settings'), + variables_settings_url: project_variables_path(@build.project, anchor: 'js-cicd-variables-settings'), build_options: javascript_build_options } } diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index 293f0a241eb..87000e8270b 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -41,7 +41,7 @@ .settings-content = render 'projects/runners/index' -%section.settings.no-animate{ class: ('expanded' if expanded), data: { qa_selector: 'variables_settings_content' } } +%section.qa-variables-settings.settings.no-animate#js-cicd-variables-settings{ class: ('expanded' if expanded), data: { qa_selector: 'variables_settings_content' } } .settings-header = render 'ci/variables/header', expanded: expanded .settings-content diff --git a/app/views/projects/triggers/_trigger.html.haml b/app/views/projects/triggers/_trigger.html.haml index 31a598ccd5e..9899cf9c6de 100644 --- a/app/views/projects/triggers/_trigger.html.haml +++ b/app/views/projects/triggers/_trigger.html.haml @@ -33,10 +33,7 @@ Never %td.text-right.trigger-actions - - take_ownership_confirmation = "By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?" - revoke_trigger_confirmation = "By revoking a trigger you will break any processes making use of it. Are you sure?" - - if trigger.owner != current_user && can?(current_user, :manage_trigger, trigger) - = link_to 'Take ownership', take_ownership_project_trigger_path(@project, trigger), data: { confirm: take_ownership_confirmation }, method: :post, class: "btn btn-default btn-sm btn-trigger-take-ownership" - if can?(current_user, :admin_trigger, trigger) = link_to edit_project_trigger_path(@project, trigger), method: :get, title: "Edit", class: "btn btn-default btn-sm" do %i.fa.fa-pencil diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index 688b600649a..b83412b5e6e 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -76,8 +76,6 @@ class BackgroundMigrationWorker # class_name - The name of the background migration that we might want to # run. def healthy_database? - return true unless Gitlab::Database.postgresql? - !Postgresql::ReplicationSlot.lag_too_great? end diff --git a/app/workers/cluster_configure_worker.rb b/app/workers/cluster_configure_worker.rb index 6f64b7ea0ab..b0e551d4e03 100644 --- a/app/workers/cluster_configure_worker.rb +++ b/app/workers/cluster_configure_worker.rb @@ -5,10 +5,6 @@ class ClusterConfigureWorker include ClusterQueue def perform(cluster_id) - Clusters::Cluster.managed.find_by_id(cluster_id).try do |cluster| - if cluster.project_type? - Clusters::RefreshService.create_or_update_namespaces_for_cluster(cluster) - end - end + # Scheduled for removal in https://gitlab.com/gitlab-org/gitlab-ce/issues/59319 end end diff --git a/app/workers/cluster_project_configure_worker.rb b/app/workers/cluster_project_configure_worker.rb index 497e57c0d0b..8f48eca4d86 100644 --- a/app/workers/cluster_project_configure_worker.rb +++ b/app/workers/cluster_project_configure_worker.rb @@ -5,8 +5,6 @@ class ClusterProjectConfigureWorker include ClusterQueue def perform(project_id) - project = Project.find(project_id) - - ::Clusters::RefreshService.create_or_update_namespaces_for_project(project) + # Scheduled for removal in https://gitlab.com/gitlab-org/gitlab-ce/issues/59319 end end |