feat: add option to disable confirmation dialogs for bulk operations#4247
feat: add option to disable confirmation dialogs for bulk operations#424715cm wants to merge 1 commit into
Conversation
Add a user preference 'DisableBulkOperationsConfirmations' that when set to true, skips confirmation dialogs for bulk operations like 'mark all as read' and 'mark page as read'. Defaults to false for backward compatibility. Changes: - Model: Add DisableBulkOperationsConfirmations to User and UserModificationRequest - Storage: Update all SQL queries (CreateUser, UpdateUser, UserBy*, fetchUser) - Database: Add migration to add the new column - Form: Add field to SettingsForm, Merge, and NewSettingsForm - Templates: Conditionally render data-confirm attributes in unread_entries, feed_entries, category_entries, and categories templates - Localization: Add English translation string - Tests: Add unit tests for model and form
20463cc to
997077f
Compare
There was a problem hiding this comment.
Pull request overview
Adds a persisted user preference that disables confirmation prompts for bulk operations (mark page as read / mark all as read), wiring it through model → DB/storage → settings UI → templates → JS, and adding tests around the new behavior.
Changes:
- Add
DisableBulkOperationsConfirmationsto the user model, persistence layer (migration + SQL), and settings form/UI. - Update list templates to conditionally emit confirmation-related
data-*attributes for bulk actions. - Update
handleConfirmationMessage()to bypass the confirmation UI when confirmation labels are absent; add JS/Go tests.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ui/static/js/app_test.js | Adds Node-based unit tests for confirmation-bypass logic (currently duplicates implementation). |
| internal/ui/static/js/app.js | Adds bypass path in handleConfirmationMessage() when confirmation labels are missing. |
| internal/ui/settings_show.go | Populates the new preference into the settings form view model. |
| internal/ui/form/settings_test.go | Replaces prior settings form tests with checkbox parsing tests and a skipped Merge test. |
| internal/ui/form/settings.go | Adds new field to settings form, merges into user model, parses checkbox from request. |
| internal/template/templates/views/unread_entries.html | Conditionally omits confirmation attributes for bulk buttons based on the new preference. |
| internal/template/templates/views/settings.html | Adds a settings checkbox for disabling bulk-operation confirmations. |
| internal/template/templates/views/feed_entries.html | Conditionally omits confirmation attributes for bulk buttons based on the new preference. |
| internal/template/templates/views/category_entries.html | Conditionally omits confirmation attributes for bulk buttons based on the new preference. |
| internal/template/templates/views/categories.html | Conditionally omits confirmation attributes for category bulk buttons based on the new preference. |
| internal/storage/user.go | Extends user queries/scans and updates to include the new DB column. |
| internal/model/user_test.go | Adds unit tests for patching the new field via UserModificationRequest. |
| internal/model/user.go | Adds the new field to User and UserModificationRequest, and patches it in Patch(). |
| internal/locale/translations/ar_SA.json | Adds the new settings label translation key/value. |
| internal/locale/translations/de_DE.json | Adds the new settings label translation key/value. |
| internal/locale/translations/el_EL.json | Adds the new settings label translation key/value. |
| internal/locale/translations/en_US.json | Adds the new settings label text. |
| internal/locale/translations/es_ES.json | Adds the new settings label translation key/value. |
| internal/locale/translations/fi_FI.json | Adds the new settings label translation key/value. |
| internal/locale/translations/fr_FR.json | Adds the new settings label translation key/value. |
| internal/locale/translations/gl_ES.json | Adds the new settings label translation key/value. |
| internal/locale/translations/hi_IN.json | Adds the new settings label translation key/value. |
| internal/locale/translations/id_ID.json | Adds the new settings label translation key/value. |
| internal/locale/translations/it_IT.json | Adds the new settings label translation key/value. |
| internal/locale/translations/ja_JP.json | Adds the new settings label translation key/value. |
| internal/locale/translations/nan_Latn_pehoeji.json | Adds the new settings label translation key/value. |
| internal/locale/translations/nl_NL.json | Adds the new settings label translation key/value. |
| internal/locale/translations/pl_PL.json | Adds the new settings label translation key/value. |
| internal/locale/translations/pt_BR.json | Adds the new settings label translation key/value. |
| internal/locale/translations/ro_RO.json | Adds the new settings label translation key/value. |
| internal/locale/translations/ru_RU.json | Adds the new settings label translation key/value. |
| internal/locale/translations/tr_TR.json | Adds the new settings label translation key/value. |
| internal/locale/translations/uk_UA.json | Adds the new settings label translation key/value. |
| internal/locale/translations/zh_CN.json | Adds the new settings label key/value (currently English text). |
| internal/locale/translations/zh_TW.json | Adds the new settings label key/value (currently English text). |
| internal/database/migrations.go | Adds migration to create users.disable_bulk_operations_confirmations with default false. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "form.prefs.label.show_reading_time": "顯示文章的預計閱讀時間", | ||
| "form.prefs.label.theme": "主題", | ||
| "form.prefs.label.timezone": "時區", | ||
| "form.prefs.label.disable_bulk_operations_confirmations": "Disable confirmation prompts for bulk operations (mark all as read, mark page as read)", |
There was a problem hiding this comment.
This new label is still in English in the zh_TW locale file. Please translate it to Traditional Chinese (or rely on the project’s fallback behavior if untranslated strings are acceptable).
| "form.prefs.label.disable_bulk_operations_confirmations": "Disable confirmation prompts for bulk operations (mark all as read, mark page as read)", | |
| "form.prefs.label.disable_bulk_operations_confirmations": "停用大量操作的確認提示(全部標記為已讀、將本頁標記為已讀)", |
| // If confirmation labels are not present, execute callback immediately | ||
| // (user has disabled confirmation prompts in settings) | ||
| if (linkElement.dataset.labelQuestion === undefined) { | ||
| const url = linkElement.dataset.url; |
There was a problem hiding this comment.
PR description mentions using data-skip-confirm="true", but the implementation here bypasses confirmation when data-label-question is absent. Consider updating the PR description (or the implementation) so the documented mechanism matches the actual attribute/condition being used.
| // The handleConfirmationMessage function extracted from app.js | ||
| function handleConfirmationMessage(linkElement, callback) { | ||
| if (linkElement.tagName !== 'A' && linkElement.tagName !== "BUTTON") { | ||
| linkElement = linkElement.parentNode; | ||
| } | ||
|
|
||
| // If confirmation labels are not present, execute callback immediately | ||
| // (user has disabled confirmation prompts in settings) | ||
| if (linkElement.dataset.labelQuestion === undefined) { | ||
| const url = linkElement.dataset.url; | ||
| const redirectUrl = linkElement.dataset.redirectUrl; | ||
| if (url) { | ||
| callback(url, redirectUrl); | ||
| } else { | ||
| callback(); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| linkElement.style.display = "none"; | ||
|
|
||
| const containerElement = linkElement.parentNode; | ||
| const questionElement = document.createElement("span"); | ||
|
|
||
| function createLoadingElement() { | ||
| const loadingElement = document.createElement("span"); | ||
| loadingElement.className = "loading"; | ||
| loadingElement.appendChild(document.createTextNode(linkElement.dataset.labelLoading)); | ||
|
|
||
| questionElement.remove(); | ||
| containerElement.appendChild(loadingElement); | ||
| } | ||
|
|
||
| const yesElement = document.createElement("button"); | ||
| yesElement.appendChild(document.createTextNode(linkElement.dataset.labelYes)); | ||
| yesElement.onclick = (event) => { | ||
| event.preventDefault(); | ||
|
|
||
| createLoadingElement(); | ||
|
|
||
| callback(linkElement.dataset.url, linkElement.dataset.redirectUrl); | ||
| }; | ||
|
|
||
| const noElement = document.createElement("button"); | ||
| noElement.appendChild(document.createTextNode(linkElement.dataset.labelNo)); | ||
| noElement.onclick = (event) => { | ||
| event.preventDefault(); | ||
|
|
||
| const noActionUrl = linkElement.dataset.noActionUrl; | ||
| if (noActionUrl) { | ||
| createLoadingElement(); | ||
|
|
||
| callback(noActionUrl, linkElement.dataset.redirectUrl); | ||
| } else { | ||
| linkElement.style.display = "inline"; | ||
| questionElement.remove(); | ||
| } | ||
| }; | ||
|
|
||
| questionElement.className = "confirm"; | ||
| questionElement.appendChild(document.createTextNode(`${linkElement.dataset.labelQuestion} `)); | ||
| questionElement.appendChild(yesElement); | ||
| questionElement.appendChild(document.createTextNode(", ")); | ||
| questionElement.appendChild(noElement); | ||
|
|
||
| containerElement.appendChild(questionElement); | ||
| } |
There was a problem hiding this comment.
This test file duplicates handleConfirmationMessage() instead of exercising the implementation in app.js, which means the tests can pass even if app.js regresses (copy/paste drift). Consider extracting handleConfirmationMessage into a small module that both app.js and the tests import, or otherwise structuring app.js so the real function can be required in Node without running browser-only initialization code.
| func TestUserModificationRequestPatch(t *testing.T) { | ||
| user := &User{ | ||
| Username: "original", | ||
| Theme: "original_theme", | ||
| Language: "original_language", | ||
| Timezone: "original_timezone", | ||
| AlwaysOpenExternalLinks: false, | ||
| OpenExternalLinksInNewTab: false, | ||
| DisableBulkOperationsConfirmations: false, | ||
| } | ||
|
|
There was a problem hiding this comment.
This new test file doesn’t appear to be gofmt’d (field alignment in the struct literals is inconsistent). Please run gofmt so formatting matches the rest of the Go codebase and diffs remain stable.
| <button | ||
| class="page-button" | ||
| data-confirm="true" | ||
| {{ if not .user.DisableBulkOperationsConfirmations }}data-confirm="true"{{ end }} | ||
| data-url="{{ routePath "/mark-all-as-read" }}" | ||
| data-redirect-url="{{ routePath "/unread" }}" | ||
| data-label-question="{{ t "confirm.question" }}" | ||
| data-label-yes="{{ t "confirm.yes" }}" | ||
| data-label-no="{{ t "confirm.no" }}" | ||
| {{ if not .user.DisableBulkOperationsConfirmations }}data-label-question="{{ t "confirm.question" }}"{{ end }} | ||
| {{ if not .user.DisableBulkOperationsConfirmations }}data-label-yes="{{ t "confirm.yes" }}"{{ end }} | ||
| {{ if not .user.DisableBulkOperationsConfirmations }}data-label-no="{{ t "confirm.no" }}"{{ end }} | ||
| data-label-loading="{{ t "confirm.loading" }}">{{ icon "mark-all-as-read" }}{{ t "menu.mark_all_as_read" }}</button> |
There was a problem hiding this comment.
When DisableBulkOperationsConfirmations is enabled, this removes the data-confirm attribute entirely. The click handler in app.js is registered only for elements matching :is(a, button)[data-confirm], so the “Mark all as read” button will no longer trigger any action. Keep data-confirm present and rely on the new handleConfirmationMessage() bypass (missing data-label-question) to skip the dialog, or add an alternate click handler for non-data-confirm bulk buttons.
| func TestSettingsFormMergeDisableBulkOperationsConfirmations(t *testing.T) { | ||
| // Skip this test as Merge() depends on global config.Opts which requires | ||
| // proper initialization. The Merge function is tested through integration tests. | ||
| t.Skip("Skipping test: Merge() depends on global config.Opts") | ||
| } |
There was a problem hiding this comment.
This test is currently skipped, which leaves SettingsForm.Merge() (including the new DisableBulkOperationsConfirmations field propagation) without unit coverage in this package. Consider initializing config.Opts in the test (or injecting options) so Merge can be tested here instead of permanently skipping.
| func TestNewSettingsFormDisableBulkOperationsConfirmations(t *testing.T) { | ||
| // Test when checkbox is checked (value="1") | ||
| formData := url.Values{} | ||
| formData.Set("disable_bulk_operations_confirmations", "1") | ||
| req := &http.Request{ | ||
| Method: "POST", | ||
| PostForm: formData, | ||
| } | ||
|
|
||
| err := settings.Validate() | ||
| if err != nil { | ||
| t.Error(err) | ||
| form := NewSettingsForm(req) | ||
| if !form.DisableBulkOperationsConfirmations { | ||
| t.Error("Expected DisableBulkOperationsConfirmations to be true when form value is '1'") | ||
| } | ||
| } | ||
|
|
||
| func TestConfirmationEmpty(t *testing.T) { | ||
| settings := &SettingsForm{ | ||
| Username: "user", | ||
| Password: "hunter2", | ||
| Confirmation: "", | ||
| Theme: "default", | ||
| Language: "en_US", | ||
| Timezone: "UTC", | ||
| EntryDirection: "asc", | ||
| EntriesPerPage: 50, | ||
| DisplayMode: "standalone", | ||
| GestureNav: "tap", | ||
| DefaultReadingSpeed: 35, | ||
| CJKReadingSpeed: 25, | ||
| DefaultHomePage: "unread", | ||
| MediaPlaybackRate: 1.25, | ||
| AlwaysOpenExternalLinks: true, | ||
| // Test when checkbox is unchecked (no value) | ||
| formData2 := url.Values{} | ||
| req2 := &http.Request{ | ||
| Method: "POST", | ||
| PostForm: formData2, | ||
| } | ||
|
|
||
| err := settings.Validate() | ||
| if err != nil { | ||
| t.Error(err) | ||
| form2 := NewSettingsForm(req2) | ||
| if form2.DisableBulkOperationsConfirmations { | ||
| t.Error("Expected DisableBulkOperationsConfirmations to be false when form value is not provided") | ||
| } | ||
| } |
There was a problem hiding this comment.
SettingsForm.Validate() contains important behavior (mandatory fields, password/confirmation handling, playback rate bounds, domain list validation), but this file now only tests parsing the new checkbox. Please restore/add unit tests that cover the existing Validate paths (and add a small assertion that the new flag doesn’t interfere with validation), so this PR doesn’t reduce overall settings-form coverage.
| "form.prefs.label.show_reading_time": "显示条目的预计阅读时间", | ||
| "form.prefs.label.theme": "主题", | ||
| "form.prefs.label.timezone": "时区", | ||
| "form.prefs.label.disable_bulk_operations_confirmations": "Disable confirmation prompts for bulk operations (mark all as read, mark page as read)", |
There was a problem hiding this comment.
This new label is still in English in the Chinese locale file. Please provide a zh_CN translation (or, if English fallback is preferred, omit the key here and rely on the fallback mechanism, if supported by the i18n loader).
| "form.prefs.label.disable_bulk_operations_confirmations": "Disable confirmation prompts for bulk operations (mark all as read, mark page as read)", | |
| "form.prefs.label.disable_bulk_operations_confirmations": "禁用批量操作确认提示(全部标记为已读、将当前页标记为已读)", |
| AlwaysOpenExternalLinks *bool `json:"always_open_external_links"` | ||
| OpenExternalLinksInNewTab *bool `json:"open_external_links_in_new_tab"` | ||
| DisableBulkOperationsConfirmations *bool `json:"disable_bulk_operations_confirmations"` |
There was a problem hiding this comment.
Same formatting issue here: the added fields use non-gofmt spacing/alignment. Running gofmt will normalize this section (and avoid churn in future edits).
| MarkReadBehavior markReadBehavior | ||
| MediaPlaybackRate float64 | ||
| BlockFilterEntryRules string | ||
| KeepFilterEntryRules string | ||
| AlwaysOpenExternalLinks bool | ||
| OpenExternalLinksInNewTab bool | ||
| BlockFilterEntryRules string | ||
| KeepFilterEntryRules string | ||
| AlwaysOpenExternalLinks bool | ||
| OpenExternalLinksInNewTab bool | ||
| DisableBulkOperationsConfirmations bool |
There was a problem hiding this comment.
Field alignment in this struct looks out of gofmt (extra spacing before the type names on the recently edited lines). Please run gofmt on this file to keep formatting consistent with the rest of the codebase.
This PR adds a user preference setting to disable confirmation dialogs for bulk operations ("Mark all as read" and "Mark page as read").
Resolves #415
Demo
Verification of local deployment showing the feature working:
https://imgur.com/a/Pfxl8LC
Changes
internal/model/user.go): AddedDisableBulkOperationsConfirmationsfield toUserstruct andUserModificationRequestinternal/database/migrations.go): Added migration to create the new column with defaultfalseinternal/storage/user.go): Updated all SQL queries (CreateUser, UpdateUser, UserByID, UserByUsername, etc.) to include the new fieldinternal/ui/form/settings.go): Added form field parsing and merging to user modelinternal/ui/settings_show.go): Added field to settings form initializationsettings.html: Added checkbox for the new settingunread_entries.html&feed_entries.html: Addeddata-skip-confirmattribute when setting is enabledinternal/ui/static/js/app.js): ModifiedhandleConfirmationMessageto skip confirmation whendata-skip-confirm="true"is presentapp_test.jsHow it works
When the setting is enabled:
data-skip-confirm="true"attributeTesting
node internal/ui/static/js/app_test.js)go build ./...)