From 27542039b6ef53ffbda86aed966bf05a62bd9215 Mon Sep 17 00:00:00 2001 From: delvh Date: Fri, 18 Feb 2022 14:47:32 +0100 Subject: [PATCH 01/46] Extract file folding into its own function --- web_src/js/features/fold-file.js | 13 +++++++++++++ web_src/js/features/repo-code.js | 6 ++---- 2 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 web_src/js/features/fold-file.js diff --git a/web_src/js/features/fold-file.js b/web_src/js/features/fold-file.js new file mode 100644 index 0000000000000..3b26e0818125b --- /dev/null +++ b/web_src/js/features/fold-file.js @@ -0,0 +1,13 @@ +import {svg} from '../svg.js'; + +/* + * Hides the file if currently present, and shows it otherwise. The actual hiding is performed using CSS. + * + * The fold arrow is the icon displayed on the upper left of the file box, especially intended for components having the 'fold-file' class. + * The file content box is the box that should be hidden or shown, especially intended for components having the 'file-content' class. + */ +export function invertFileFolding(fileContentBox, foldArrow) { + const newFold = fileContentBox.getAttribute('data-folded') !== 'true'; + foldArrow.innerHTML = svg(`octicon-chevron-${newFold ? 'right' : 'down'}`, 18); + fileContentBox.setAttribute('data-folded', String(newFold)); +} diff --git a/web_src/js/features/repo-code.js b/web_src/js/features/repo-code.js index a4b6e433a5927..acae15822b904 100644 --- a/web_src/js/features/repo-code.js +++ b/web_src/js/features/repo-code.js @@ -1,5 +1,6 @@ import $ from 'jquery'; import {svg} from '../svg.js'; +import {invertFileFolding} from './fold-file.js'; function changeHash(hash) { if (window.history.pushState) { @@ -134,10 +135,7 @@ export function initRepoCodeView() { }).trigger('hashchange'); } $(document).on('click', '.fold-file', ({currentTarget}) => { - const box = currentTarget.closest('.file-content'); - const folded = box.getAttribute('data-folded') !== 'true'; - currentTarget.innerHTML = svg(`octicon-chevron-${folded ? 'right' : 'down'}`, 18); - box.setAttribute('data-folded', String(folded)); + invertFileFolding(currentTarget.closest('.file-content'), currentTarget); }); $(document).on('click', '.blob-excerpt', async ({currentTarget}) => { const url = currentTarget.getAttribute('data-url'); From eb22ccbd8d114884023d1a4957a20bb5941bfb2a Mon Sep 17 00:00:00 2001 From: delvh Date: Fri, 18 Feb 2022 18:32:09 +0100 Subject: [PATCH 02/46] Add automatic frontend file folding on load --- models/migrations/v210.go | 2 +- models/migrations/v210_test.go | 1 + web_src/js/features/file-fold.js | 34 ++++++++++++++++++++++++++++++++ web_src/js/features/fold-file.js | 13 ------------ web_src/js/features/repo-code.js | 2 +- web_src/js/index.js | 2 ++ 6 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 web_src/js/features/file-fold.js delete mode 100644 web_src/js/features/fold-file.js diff --git a/models/migrations/v210.go b/models/migrations/v210.go index cf50760b92c33..9da8ca9db6643 100644 --- a/models/migrations/v210.go +++ b/models/migrations/v210.go @@ -11,8 +11,8 @@ import ( "strings" "code.gitea.io/gitea/modules/timeutil" - "github.com/tstranex/u2f" + "github.com/tstranex/u2f" "xorm.io/xorm" "xorm.io/xorm/schemas" ) diff --git a/models/migrations/v210_test.go b/models/migrations/v210_test.go index 3e10b3ce80a25..70dbe61b06eb7 100644 --- a/models/migrations/v210_test.go +++ b/models/migrations/v210_test.go @@ -8,6 +8,7 @@ import ( "testing" "code.gitea.io/gitea/modules/timeutil" + "github.com/stretchr/testify/assert" "xorm.io/xorm/schemas" ) diff --git a/web_src/js/features/file-fold.js b/web_src/js/features/file-fold.js new file mode 100644 index 0000000000000..da8be687811c8 --- /dev/null +++ b/web_src/js/features/file-fold.js @@ -0,0 +1,34 @@ +import {svg} from '../svg.js'; + + +// Hides the file if newFold is true, and shows it otherwise. The actual hiding is performed using CSS. +// +// The fold arrow is the icon displayed on the upper left of the file box, especially intended for components having the 'fold-file' class. +// The file content box is the box that should be hidden or shown, especially intended for components having the 'file-content' class. +// +function setFileFolding(fileContentBox, foldArrow, newFold) { + foldArrow.innerHTML = svg(`octicon-chevron-${newFold ? 'right' : 'down'}`, 18); + fileContentBox.setAttribute('data-folded', String(newFold)); +} + +// Like `setFileFolding`, except that it automatically inverts the current file folding state. +export function invertFileFolding(fileContentBox, foldArrow) { + setFileFolding(fileContentBox, foldArrow, fileContentBox.getAttribute('data-folded') !== 'true'); +} + +// Folds every file stored in `window.config.pageData.viewedFiles` according to the value that is persisted there. +// +// All other files not present there are simply ignored. +export function resetFileFolding() { + const viewedFiles = window.config.pageData.viewedFiles; + if (!viewedFiles) return; + + const filenameAttribute = 'data-new-filename'; + for (const file of document.querySelectorAll('.file-content')) { + if (file.hasAttribute(filenameAttribute)) { + const filename = file.getAttribute(filenameAttribute); + setFileFolding(file, file.querySelector('.fold-file'), filename in viewedFiles || viewedFiles.includes(filename)); // because of this OR, arrays as well as objects are supported + } + } +} + diff --git a/web_src/js/features/fold-file.js b/web_src/js/features/fold-file.js deleted file mode 100644 index 3b26e0818125b..0000000000000 --- a/web_src/js/features/fold-file.js +++ /dev/null @@ -1,13 +0,0 @@ -import {svg} from '../svg.js'; - -/* - * Hides the file if currently present, and shows it otherwise. The actual hiding is performed using CSS. - * - * The fold arrow is the icon displayed on the upper left of the file box, especially intended for components having the 'fold-file' class. - * The file content box is the box that should be hidden or shown, especially intended for components having the 'file-content' class. - */ -export function invertFileFolding(fileContentBox, foldArrow) { - const newFold = fileContentBox.getAttribute('data-folded') !== 'true'; - foldArrow.innerHTML = svg(`octicon-chevron-${newFold ? 'right' : 'down'}`, 18); - fileContentBox.setAttribute('data-folded', String(newFold)); -} diff --git a/web_src/js/features/repo-code.js b/web_src/js/features/repo-code.js index acae15822b904..a47ed6a0e6a58 100644 --- a/web_src/js/features/repo-code.js +++ b/web_src/js/features/repo-code.js @@ -1,6 +1,6 @@ import $ from 'jquery'; import {svg} from '../svg.js'; -import {invertFileFolding} from './fold-file.js'; +import {invertFileFolding} from './file-fold.js'; function changeHash(hash) { if (window.history.pushState) { diff --git a/web_src/js/index.js b/web_src/js/index.js index c9a7949d5a308..208d1c518db7c 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -75,6 +75,7 @@ import {initRepoBranchButton} from './features/repo-branch.js'; import {initCommonOrganization} from './features/common-organization.js'; import {initRepoWikiForm} from './features/repo-wiki.js'; import {initRepoCommentForm, initRepository} from './features/repo-legacy.js'; +import {resetFileFolding} from './features/file-fold.js'; // Silence fomantic's error logging when tabs are used without a target content element $.fn.tab.settings.silent = true; @@ -167,4 +168,5 @@ $(document).ready(() => { initUserAuthWebAuthn(); initUserAuthWebAuthnRegister(); initUserSettings(); + resetFileFolding(); }); From 20791c7851e85e4f4ce67b82af06de876878e0df Mon Sep 17 00:00:00 2001 From: delvh Date: Sat, 26 Feb 2022 01:50:56 +0100 Subject: [PATCH 03/46] Add "Viewed" checkbox and "Already changed" label in the frontend They are even styled already! --- options/locale/locale_en-US.ini | 2 ++ services/gitdiff/gitdiff.go | 40 +++++++++++++++++---------------- templates/repo/diff/box.tmpl | 12 ++++++++-- web_src/less/_base.less | 7 ++++++ web_src/less/_repository.less | 10 +++++++++ 5 files changed, 50 insertions(+), 21 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index f52fef3c0590b..b1ce5eb725cf1 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1480,6 +1480,8 @@ pulls.new = New Pull Request pulls.view = View Pull Request pulls.compare_changes = New Pull Request pulls.compare_changes_desc = Select the branch to merge into and the branch to pull from. +pulls.has_viewed_file = Viewed +pulls.has_changed_since_last_review = Changed since your last review pulls.compare_base = merge into pulls.compare_compare = pull from pulls.switch_comparison_type = Switch comparison type diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 58c25ff98f827..d20694bd56611 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -602,25 +602,27 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine) Dif // DiffFile represents a file diff. type DiffFile struct { - Name string - OldName string - Index int - Addition, Deletion int - Type DiffFileType - IsCreated bool - IsDeleted bool - IsBin bool - IsLFSFile bool - IsRenamed bool - IsAmbiguous bool - IsSubmodule bool - Sections []*DiffSection - IsIncomplete bool - IsIncompleteLineTooLong bool - IsProtected bool - IsGenerated bool - IsVendored bool - Language string + Name string + OldName string + Index int + Addition, Deletion int + Type DiffFileType + IsCreated bool + IsDeleted bool + IsBin bool + IsLFSFile bool + IsRenamed bool + IsAmbiguous bool + IsSubmodule bool + Sections []*DiffSection + IsIncomplete bool + IsIncompleteLineTooLong bool + IsProtected bool + IsGenerated bool + IsVendored bool + IsViewed bool // User specific + HasChangedSinceLastReview bool // User specific + Language string } // GetType returns type of diff file. diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index f115a5f49941e..dba151b3b34a3 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -61,7 +61,7 @@

- {{if $file.IsGenerated}} + {{if or $file.IsGenerated $file.IsViewed}} {{svg "octicon-chevron-right" 18}} {{else}} {{svg "octicon-chevron-down" 18}} @@ -105,9 +105,17 @@ {{$.i18n.Tr "repo.diff.view_file"}} {{end}} {{end}} + {{if and $.IsSigned $.PageIsPullFiles}} +
+ {{if $file.HasChangedSinceLastReview}} + {{$.i18n.Tr "repo.pulls.has_changed_since_last_review"}} + {{end}} + + + {{end}}

-
+
{{if or $file.IsIncomplete $file.IsBin}}
diff --git a/web_src/less/_base.less b/web_src/less/_base.less index 55439d6cea28a..f76113ddefae5 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -2188,3 +2188,10 @@ table th[data-sortt-desc] { height: 15px; } } + +.changed-since-last-review { + margin: 0px 5px; + padding: 0px 3px; + border: 2px var(--color-primary-light-3) solid; + border-radius:7px; +} diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index 8b912ce90d861..26341777f5c6b 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -3129,6 +3129,16 @@ td.blob-excerpt { } } +.viewed-file-checkbox { + margin: 0px 3px; + padding: 0px 3px; + border-radius: 3px; +} + +.viewed-file-checked-checkbox { + background-color: var(--color-primary-light-4); +} + /* prevent page shaking on language bar click */ .repository-summary-language-stats { height: 48px; From 0574ee986f75e8f519129ac071b8ba0d92d848d2 Mon Sep 17 00:00:00 2001 From: delvh Date: Sat, 26 Feb 2022 02:09:44 +0100 Subject: [PATCH 04/46] Remove unneeded function --- web_src/js/features/file-fold.js | 16 ---------------- web_src/js/index.js | 2 -- web_src/less/_base.less | 6 +++--- web_src/less/_repository.less | 4 ++-- 4 files changed, 5 insertions(+), 23 deletions(-) diff --git a/web_src/js/features/file-fold.js b/web_src/js/features/file-fold.js index da8be687811c8..639b0f236d34a 100644 --- a/web_src/js/features/file-fold.js +++ b/web_src/js/features/file-fold.js @@ -16,19 +16,3 @@ export function invertFileFolding(fileContentBox, foldArrow) { setFileFolding(fileContentBox, foldArrow, fileContentBox.getAttribute('data-folded') !== 'true'); } -// Folds every file stored in `window.config.pageData.viewedFiles` according to the value that is persisted there. -// -// All other files not present there are simply ignored. -export function resetFileFolding() { - const viewedFiles = window.config.pageData.viewedFiles; - if (!viewedFiles) return; - - const filenameAttribute = 'data-new-filename'; - for (const file of document.querySelectorAll('.file-content')) { - if (file.hasAttribute(filenameAttribute)) { - const filename = file.getAttribute(filenameAttribute); - setFileFolding(file, file.querySelector('.fold-file'), filename in viewedFiles || viewedFiles.includes(filename)); // because of this OR, arrays as well as objects are supported - } - } -} - diff --git a/web_src/js/index.js b/web_src/js/index.js index 208d1c518db7c..c9a7949d5a308 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -75,7 +75,6 @@ import {initRepoBranchButton} from './features/repo-branch.js'; import {initCommonOrganization} from './features/common-organization.js'; import {initRepoWikiForm} from './features/repo-wiki.js'; import {initRepoCommentForm, initRepository} from './features/repo-legacy.js'; -import {resetFileFolding} from './features/file-fold.js'; // Silence fomantic's error logging when tabs are used without a target content element $.fn.tab.settings.silent = true; @@ -168,5 +167,4 @@ $(document).ready(() => { initUserAuthWebAuthn(); initUserAuthWebAuthnRegister(); initUserSettings(); - resetFileFolding(); }); diff --git a/web_src/less/_base.less b/web_src/less/_base.less index f76113ddefae5..e03154b811411 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -2190,8 +2190,8 @@ table th[data-sortt-desc] { } .changed-since-last-review { - margin: 0px 5px; - padding: 0px 3px; + margin: 0 5px; + padding: 0 3px; border: 2px var(--color-primary-light-3) solid; - border-radius:7px; + border-radius: 7px; } diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index 26341777f5c6b..eb47e2aa3e1cf 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -3130,8 +3130,8 @@ td.blob-excerpt { } .viewed-file-checkbox { - margin: 0px 3px; - padding: 0px 3px; + margin: 0 3px; + padding: 0 3px; border-radius: 3px; } From e24b55cf6d7ccb57a52fa3d00b261ee4596a1285 Mon Sep 17 00:00:00 2001 From: delvh Date: Sat, 26 Feb 2022 17:55:32 +0100 Subject: [PATCH 05/46] Finish frontend completely except for sending the event to the backend Additionally clean up CSS a little bit --- options/locale/locale_en-US.ini | 2 ++ services/gitdiff/gitdiff.go | 10 +++--- templates/repo/blame.tmpl | 2 +- templates/repo/diff/box.tmpl | 7 ++-- .../repo/pulls/viewed_files_summary.tmpl | 4 +++ web_src/js/features/file-fold.js | 2 +- web_src/js/features/pull-view-file.js | 35 +++++++++++++++++++ web_src/js/index.js | 2 ++ web_src/less/_base.less | 13 ++++--- 9 files changed, 65 insertions(+), 12 deletions(-) create mode 100644 templates/repo/pulls/viewed_files_summary.tmpl create mode 100644 web_src/js/features/pull-view-file.js diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b1ce5eb725cf1..12e01a8bb880a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1482,6 +1482,8 @@ pulls.compare_changes = New Pull Request pulls.compare_changes_desc = Select the branch to merge into and the branch to pull from. pulls.has_viewed_file = Viewed pulls.has_changed_since_last_review = Changed since your last review +pulls.viewed_files_label = %[1]d / %[2]d files viewed +pulls.viewed_files_label_template = $1 / $2 files viewed pulls.compare_base = merge into pulls.compare_compare = pull from pulls.switch_comparison_type = Switch comparison type diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index d20694bd56611..8cf8334ace478 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -679,10 +679,12 @@ func getCommitFileLineCount(commit *git.Commit, filePath string) int { // Diff represents a difference between two git trees. type Diff struct { - Start, End string - NumFiles, TotalAddition, TotalDeletion int - Files []*DiffFile - IsIncomplete bool + Start, End string + NumFiles int + TotalAddition, TotalDeletion int + Files []*DiffFile + IsIncomplete bool + NumViewedFiles int // user-specific } // LoadComments loads comments into each line diff --git a/templates/repo/blame.tmpl b/templates/repo/blame.tmpl index 3dc3522275b17..6ed24a47a5e0e 100644 --- a/templates/repo/blame.tmpl +++ b/templates/repo/blame.tmpl @@ -27,7 +27,7 @@ {{range $row := .BlameRows}} - +
diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index dba151b3b34a3..6bd6e111d0ace 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -18,6 +18,9 @@ {{svg "octicon-diff" 16 "mr-2"}}{{.i18n.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
+ {{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}} + {{template "repo/pulls/viewed_files_summary" .}} + {{end}} {{template "repo/diff/whitespace_dropdown" .}} {{template "repo/diff/options_dropdown" .}} {{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}} @@ -105,8 +108,8 @@ {{$.i18n.Tr "repo.diff.view_file"}} {{end}} {{end}} - {{if and $.IsSigned $.PageIsPullFiles}} -
+ {{if and $.IsSigned $.PageIsPullFiles (not $.IsArchived)}} +
{{if $file.HasChangedSinceLastReview}} {{$.i18n.Tr "repo.pulls.has_changed_since_last_review"}} {{end}} diff --git a/templates/repo/pulls/viewed_files_summary.tmpl b/templates/repo/pulls/viewed_files_summary.tmpl new file mode 100644 index 0000000000000..1ba6a1f08f60c --- /dev/null +++ b/templates/repo/pulls/viewed_files_summary.tmpl @@ -0,0 +1,4 @@ + + diff --git a/web_src/js/features/file-fold.js b/web_src/js/features/file-fold.js index 639b0f236d34a..8fd6df44c3608 100644 --- a/web_src/js/features/file-fold.js +++ b/web_src/js/features/file-fold.js @@ -6,7 +6,7 @@ import {svg} from '../svg.js'; // The fold arrow is the icon displayed on the upper left of the file box, especially intended for components having the 'fold-file' class. // The file content box is the box that should be hidden or shown, especially intended for components having the 'file-content' class. // -function setFileFolding(fileContentBox, foldArrow, newFold) { +export function setFileFolding(fileContentBox, foldArrow, newFold) { foldArrow.innerHTML = svg(`octicon-chevron-${newFold ? 'right' : 'down'}`, 18); fileContentBox.setAttribute('data-folded', String(newFold)); } diff --git a/web_src/js/features/pull-view-file.js b/web_src/js/features/pull-view-file.js new file mode 100644 index 0000000000000..84c983765c7d2 --- /dev/null +++ b/web_src/js/features/pull-view-file.js @@ -0,0 +1,35 @@ +import {setFileFolding} from './file-fold.js'; + +const viewedStyleClass = 'viewed-file-checked-checkbox'; + +// Initializes a listener for all children of the given html element +// (for example 'document' in the most basic case) +// to watch for changes of viewed-file checkboxes +export function initViewedCheckboxListenerFor(element) { + for (const wholeCheckbox of element.querySelectorAll('.viewed-file-checkbox')) { + // The checkbox consists of a div containing the real checkbox, and a label, + // hence the actual checkbox first has to be found + const checkbox = wholeCheckbox.querySelector('input[type=checkbox]'); + checkbox.addEventListener('change', function() { + // Mark visually as viewed - will especially change the background of the whole block + if (this.checked) { + wholeCheckbox.classList.add(viewedStyleClass); + window.config.pageData.numberOfViewedFiles++; + } else { + wholeCheckbox.classList.remove(viewedStyleClass); + window.config.pageData.numberOfViewedFiles--; + } + + // Update viewed-files summary + document.getElementById('viewed-files-summary').setAttribute('value', window.config.pageData.numberOfViewedFiles); + const summaryLabel = document.getElementById('viewed-files-summary-label'); + summaryLabel.innerHTML = summaryLabel.getAttribute('data-text-changed-template') + .replace('$1', window.config.pageData.numberOfViewedFiles) + .replace('$2', window.config.pageData.numberOfFiles); + + // Fold the file accordingly + const parentBox = wholeCheckbox.closest('.diff-file-header'); + setFileFolding(parentBox.closest('.file-content'), parentBox.querySelector('.fold-file'), this.checked); + }); + } +} diff --git a/web_src/js/index.js b/web_src/js/index.js index c9a7949d5a308..c5b2800c61588 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -63,6 +63,7 @@ import { initRepoSettingsCollaboration, initRepoSettingSearchTeamBox, } from './features/repo-settings.js'; +import {initViewedCheckboxListenerFor} from './features/pull-view-file.js'; import {initOrgTeamSearchRepoBox, initOrgTeamSettings} from './features/org-team.js'; import {initUserAuthWebAuthn, initUserAuthWebAuthnRegister} from './features/user-auth-webauthn.js'; import {initRepoRelease, initRepoReleaseEditor} from './features/repo-release.js'; @@ -167,4 +168,5 @@ $(document).ready(() => { initUserAuthWebAuthn(); initUserAuthWebAuthnRegister(); initUserSettings(); + initViewedCheckboxListenerFor(document); }); diff --git a/web_src/less/_base.less b/web_src/less/_base.less index e03154b811411..b2cb3d1c0e26c 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -764,6 +764,15 @@ a.ui.card:hover, } } +.unselectable { + -webkit-touch-callout: none; + -webkit-user-select: none; + -khtml-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; +} + .right.stackable.menu { // responsive fix: this makes sure that the right menu when the page // is on mobile view will have elements stacked on top of each other. @@ -1621,10 +1630,6 @@ a.ui.label:hover { padding: 0 !important; background: var(--color-code-sidebar-bg); width: 1%; - -moz-user-select: none; - -ms-user-select: none; - -webkit-user-select: none; - user-select: none; .blame-info { width: 350px; From 35afb823fcdd8846e298863197361313c686c53c Mon Sep 17 00:00:00 2001 From: delvh Date: Sun, 27 Feb 2022 01:05:43 +0100 Subject: [PATCH 06/46] Inform the server about view changes - frontend is now complete --- routers/web/repo/pull_viewed_files.go | 12 ++++++++++++ routers/web/web.go | 1 + templates/repo/diff/box.tmpl | 10 +++++++--- web_src/js/features/pull-view-file.js | 28 +++++++++++++++++++-------- web_src/less/_repository.less | 4 ++-- 5 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 routers/web/repo/pull_viewed_files.go diff --git a/routers/web/repo/pull_viewed_files.go b/routers/web/repo/pull_viewed_files.go new file mode 100644 index 0000000000000..05bc612ca49da --- /dev/null +++ b/routers/web/repo/pull_viewed_files.go @@ -0,0 +1,12 @@ +// Copyright 2022 The Gitea Authors. +// All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "code.gitea.io/gitea/modules/context" +) + +func UpdateViewedFiles(ctx *context.Context) {} diff --git a/routers/web/web.go b/routers/web/web.go index d8c197fb967e2..24a482879ce3d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -741,6 +741,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/content", repo.UpdateIssueContent) m.Post("/watch", repo.IssueWatch) m.Post("/ref", repo.UpdateIssueRef) + m.Post("/viewed-files", repo.UpdateViewedFiles) m.Group("/dependency", func() { m.Post("/add", repo.AddDependency) m.Post("/delete", repo.RemoveDependency) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 6bd6e111d0ace..acd3d32e3d0d6 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -109,12 +109,16 @@ {{end}} {{end}} {{if and $.IsSigned $.PageIsPullFiles (not $.IsArchived)}} -
+
{{if $file.HasChangedSinceLastReview}} {{$.i18n.Tr "repo.pulls.has_changed_since_last_review"}} {{end}} - - +
+ {{$.CsrfTokenHtml}} + + + +
{{end}}
diff --git a/web_src/js/features/pull-view-file.js b/web_src/js/features/pull-view-file.js index 84c983765c7d2..4be7824b1d9ba 100644 --- a/web_src/js/features/pull-view-file.js +++ b/web_src/js/features/pull-view-file.js @@ -1,22 +1,24 @@ import {setFileFolding} from './file-fold.js'; -const viewedStyleClass = 'viewed-file-checked-checkbox'; +const viewedStyleClass = 'viewed-file-checked-form'; // Initializes a listener for all children of the given html element // (for example 'document' in the most basic case) // to watch for changes of viewed-file checkboxes export function initViewedCheckboxListenerFor(element) { - for (const wholeCheckbox of element.querySelectorAll('.viewed-file-checkbox')) { - // The checkbox consists of a div containing the real checkbox, and a label, + for (const form of element.querySelectorAll('.viewed-file-form')) { + + // The checkbox consists of a form containing the real checkbox and a label, // hence the actual checkbox first has to be found - const checkbox = wholeCheckbox.querySelector('input[type=checkbox]'); + const checkbox = form.querySelector('input[type=checkbox]'); checkbox.addEventListener('change', function() { - // Mark visually as viewed - will especially change the background of the whole block + + // Mark the file as viewed visually - will especially change the background if (this.checked) { - wholeCheckbox.classList.add(viewedStyleClass); + form.classList.add(viewedStyleClass); window.config.pageData.numberOfViewedFiles++; } else { - wholeCheckbox.classList.remove(viewedStyleClass); + form.classList.remove(viewedStyleClass); window.config.pageData.numberOfViewedFiles--; } @@ -27,8 +29,18 @@ export function initViewedCheckboxListenerFor(element) { .replace('$1', window.config.pageData.numberOfViewedFiles) .replace('$2', window.config.pageData.numberOfFiles); + // Unfortunately, form.submit() would attempt to redirect, so we have to workaround that + // Because unchecked checkboxes are also not sent, we have to unset the value and use the fallback hidden input as sent value + const previousCheckedState = this.checked; + this.checked = false; + this.previousElementSibling.setAttribute('value', previousCheckedState); + const request = new XMLHttpRequest(); + request.open('POST', form.getAttribute('action')); + request.send(new FormData(form)); + this.checked = previousCheckedState; + // Fold the file accordingly - const parentBox = wholeCheckbox.closest('.diff-file-header'); + const parentBox = form.closest('.diff-file-header'); setFileFolding(parentBox.closest('.file-content'), parentBox.querySelector('.fold-file'), this.checked); }); } diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index eb47e2aa3e1cf..1206b68b801c7 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -3129,13 +3129,13 @@ td.blob-excerpt { } } -.viewed-file-checkbox { +.viewed-file-form { margin: 0 3px; padding: 0 3px; border-radius: 3px; } -.viewed-file-checked-checkbox { +.viewed-file-checked-form { background-color: var(--color-primary-light-4); } From 59f945d63b93baccc3df2c08e7a93634b052fd18 Mon Sep 17 00:00:00 2001 From: delvh Date: Mon, 28 Feb 2022 23:41:49 +0100 Subject: [PATCH 07/46] Persist PR Reviews in the database Additionally throw out redundant resource string --- models/migrations/v211.go | 23 ++++++ models/pull_review.go | 81 +++++++++++++++++++ options/locale/locale_en-US.ini | 1 - routers/web/repo/pull_viewed_files.go | 41 +++++++++- templates/repo/diff/box.tmpl | 6 +- .../repo/pulls/viewed_files_summary.tmpl | 2 +- web_src/js/features/pull-view-file.js | 6 +- 7 files changed, 151 insertions(+), 9 deletions(-) create mode 100644 models/migrations/v211.go create mode 100644 models/pull_review.go diff --git a/models/migrations/v211.go b/models/migrations/v211.go new file mode 100644 index 0000000000000..cfae6d20ba81b --- /dev/null +++ b/models/migrations/v211.go @@ -0,0 +1,23 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "code.gitea.io/gitea/modules/timeutil" + "xorm.io/xorm" +) + +func addPRReviewedFiles(x *xorm.Engine) error { + type PRReview struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"` + ViewedFiles map[string]bool `xorm:"TEXT JSON"` + CommitSHA string `xorm:"NOT NULL UNIQUE(pull_commit_user)"` + PullID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated"` + } + + return x.Sync2(new(PRReview)) +} diff --git a/models/pull_review.go b/models/pull_review.go new file mode 100644 index 0000000000000..9c6fea39912eb --- /dev/null +++ b/models/pull_review.go @@ -0,0 +1,81 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. +package models + +import ( + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/timeutil" +) + +type PRReview struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"` + ViewedFiles map[string]bool `xorm:"TEXT JSON"` // Stores for each of the changed files of a PR whether they have been viewed or not + CommitSHA string `xorm:"NOT NULL UNIQUE(pull_commit_user)"` // Which commit was the head commit for the review? + PullID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"` // Which PR was the review on? + UpdatedUnix timeutil.TimeStamp `xorm:"updated"` // Is an accurate indicator of the order of commits as we do not expect it to be possible to make reviews on previous commits +} + +func init() { + db.RegisterModel(new(PRReview)) +} + +// GetReview returns the PRReview with all given values prefilled, whether or not it exists in the database. +// If the review didn't exist before in the database, it won't afterwards either. +// The returned boolean shows whether the review exists in the database +func GetReview(userID, pullID int64, commitSHA string) (*PRReview, bool, error) { + review := &PRReview{UserID: userID, CommitSHA: commitSHA, PullID: pullID} + has, err := db.GetEngine(db.DefaultContext).Get(review) + return review, has, err +} + +// UpdateReview updates the given review inside the database, regardless of whether it existed before or not +// The given map of viewed files will be merged with the previous review, if present +func UpdateReview(userID, pullID int64, commitSHA string, viewedFiles map[string]bool) error { + review, exists, err := GetReview(userID, pullID, commitSHA) + engine := db.GetEngine(db.DefaultContext) + if previousReview, err := getNewestReviewApartFrom(commitSHA, userID, pullID); err != nil { + return err + + // Overwrite the viewed files of the previous review if present + } else if previousReview != nil && previousReview.ViewedFiles != nil { + var newlyViewedFiles = viewedFiles + viewedFiles = previousReview.ViewedFiles + for file, viewed := range newlyViewedFiles { + viewedFiles[file] = viewed + } + } + review.ViewedFiles = viewedFiles + if err != nil { + return err + + // Insert or Update review + } else if !exists { + _, err := engine.Insert(review) + return err + } + _, err = engine.ID(review.ID).Update(review) + return err +} + +// GetNewestReview gets the newest review of the current user in the current PR. +// The returned PR Review will be nil if the user has not yet reviewed this PR. +func GetNewestReview(userID, pullID int64) (*PRReview, error) { + var review *PRReview + has, err := db.GetEngine(db.DefaultContext).Where("user_id = ?", userID).And("pull_id = ?", pullID).OrderBy("updated_unix DESC").Limit(1).Get(&review) + if err != nil || !has { + return nil, err + } + return review, err +} + +// getNewestReviewApartFrom is like GetNewestReview, except that the second newest review will be returned if the newest review points at the given commit +func getNewestReviewApartFrom(commitSHA string, userID, pullID int64) (*PRReview, error) { + var review *PRReview + has, err := db.GetEngine(db.DefaultContext).Where("user_id = ?", userID).And("pull_id = ?", pullID).And("commit_sha != ?", commitSHA).OrderBy("updated_unix DESC").Limit(1).Get(&review) + if err != nil || !has { + return nil, err + } + return review, err +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 12e01a8bb880a..4c0db1aa366e6 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1483,7 +1483,6 @@ pulls.compare_changes_desc = Select the branch to merge into and the branch to p pulls.has_viewed_file = Viewed pulls.has_changed_since_last_review = Changed since your last review pulls.viewed_files_label = %[1]d / %[2]d files viewed -pulls.viewed_files_label_template = $1 / $2 files viewed pulls.compare_base = merge into pulls.compare_compare = pull from pulls.switch_comparison_type = Switch comparison type diff --git a/routers/web/repo/pull_viewed_files.go b/routers/web/repo/pull_viewed_files.go index 05bc612ca49da..d5c859faca334 100644 --- a/routers/web/repo/pull_viewed_files.go +++ b/routers/web/repo/pull_viewed_files.go @@ -6,7 +6,46 @@ package repo import ( + "strconv" + + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" ) -func UpdateViewedFiles(ctx *context.Context) {} +const headCommitKey = "_headCommitID" + +func UpdateViewedFiles(ctx *context.Context) { + pull := checkPullInfo(ctx).PullRequest + if ctx.Written() { + return + } + updatedFiles := make(map[string]bool, len(ctx.Req.Form)) + headCommitID := "" + + // Collect all files and their viewed state + for file, values := range ctx.Req.Form { + for _, viewedString := range values { + viewed, err := strconv.ParseBool(viewedString) + + // Ignore fields that do not parse as a boolean, i.e. the CSRF token + if err != nil { + + // Prevent invalid reviews by specifically supplying the commit the user viewed the file under + if file == headCommitKey { + headCommitID = viewedString + } + continue + } + updatedFiles[file] = viewed + } + } + + // No head commit ID was supplied - expect the review to have been now + if headCommitID == "" { + headCommitID = pull.HeadCommitID + } + + if err := models.UpdateReview(ctx.User.ID, pull.ID, headCommitID, updatedFiles); err != nil { + ctx.ServerError("UpdateReview", err) + } +} diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index acd3d32e3d0d6..60228140811d5 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -115,8 +115,10 @@ {{end}}
{{$.CsrfTokenHtml}} - - + + + +
{{end}} diff --git a/templates/repo/pulls/viewed_files_summary.tmpl b/templates/repo/pulls/viewed_files_summary.tmpl index 1ba6a1f08f60c..eab202471c852 100644 --- a/templates/repo/pulls/viewed_files_summary.tmpl +++ b/templates/repo/pulls/viewed_files_summary.tmpl @@ -1,4 +1,4 @@ -