github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-01-18 13:43:16 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: Wait for bookmark save before allowing menu button click (#26626)

Browse Source 
This fixes a timing issue where, if a user (or the CI) was
on a slow network connection, clicking one of the bookmark
menu options would cause an error because we hadn't yet received
the response from the server after creating the bookmark.

It should be very smooth most of the times because (paraphrasing j.jaffeux):

a) Most likely when user clicks it’s already saved
b) If it’s not saved when user clicks, it should already be almost done so
   the perceived wait when click the reminder option should be rather short
This commit is contained in:
Martin Brennan 2024-04-15 22:45:11 +10:00 committed by GitHub
parent 326da84373
commit 8640d3c82d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 38 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
app/assets/javascripts/discourse/app/components/bookmark-menu.gjs
View File

@ -28,6 +28,7 @@ export default class BookmarkMenu extends Component {
bookmarkManager = this.args.bookmarkManager;
timezone = this.currentUser?.user_option?.timezone || moment.tz.guess();
timeShortcuts = timeShortcuts(this.timezone);
bookmarkCreatePromise = null;
@action
setReminderShortcuts() {
@ -79,9 +80,10 @@ export default class BookmarkMenu extends Component {
}
@action
async onBookmark() {
try {
await this.bookmarkManager.create();
onBookmark() {
this.bookmarkCreatePromise = this.bookmarkManager.create();
this.bookmarkCreatePromise
.then(() => {
// We show the menu with Edit/Delete options if the bokmark exists,
// so this "quicksave" will do nothing in that case.
// NOTE: Need a nicer way to handle this; otherwise as soon as you save
@ -92,9 +94,11 @@ export default class BookmarkMenu extends Component {
views: ["mobile"],
data: { message: I18n.t("bookmarks.bookmarked_success") },
});
} catch (error) {
popupAjaxError(error);
}
})
.catch((error) => popupAjaxError(error))
.finally(() => {
this.bookmarkCreatePromise = null;
});
}
@action
@ -140,6 +144,10 @@ export default class BookmarkMenu extends Component {
@action
async onChooseReminderOption(option) {
if (this.bookmarkCreatePromise) {
await this.bookmarkCreatePromise;
}
if (option.id === TIME_SHORTCUT_TYPES.CUSTOM) {
this._openBookmarkModal();
} else {

16
spec/system/bookmarks_spec.rb
View File

@ -7,6 +7,7 @@ describe "Bookmarking posts and topics", type: :system do
fab!(:post_2) { Fabricate(:post, topic: topic, raw: "Some interesting post content") }
let(:timezone) { "Australia/Brisbane" }
let(:cdp) { PageObjects::CDP.new }
let(:topic_page) { PageObjects::Pages::Topic.new }
let(:bookmark_modal) { PageObjects::Modals::Bookmark.new }
let(:bookmark_menu) { PageObjects::Components::BookmarkMenu.new }
@ -18,9 +19,11 @@ describe "Bookmarking posts and topics", type: :system do
def visit_topic_and_open_bookmark_menu(post, expand_actions: true)
topic_page.visit_topic(topic)
open_bookmark_menu(post, expand_actions: expand_actions)
end
def open_bookmark_menu(post, expand_actions: true)
topic_page.expand_post_actions(post) if expand_actions
topic_page.click_post_action_button(post, :bookmark)
end
@ -80,6 +83,17 @@ describe "Bookmarking posts and topics", type: :system do
)
end
it "opens the bookmark modal with the Custom... option only after the bookmark saves on slow connections" do
topic_page.visit_topic(topic)
cdp.with_slow_upload do
open_bookmark_menu(post)
bookmark_menu.click_menu_option("custom")
end
expect(bookmark_modal).to be_open
end
describe "topic level bookmarks" do
it "allows the topic to be bookmarked" do
topic_page.visit_topic(topic)