fix(bookmarks): honor shared-bookmark rules in getList and getByLabel - #20347
Open
cavalcante-willian wants to merge 1 commit into
Open
fix(bookmarks): honor shared-bookmark rules in getList and getByLabel#20347cavalcante-willian wants to merge 1 commit into
cavalcante-willian wants to merge 1 commit into
Conversation
BookmarkRepository::getList() only matched bookmarks scoped to the exact requested database, so bookmarks created without a database context (dbase = '', e.g. via the Console, which does not require a database to be selected) never showed up when browsing a specific database's bookmark list. The dbase filter now also matches an empty dbase. BookmarkRepository::getByLabel() ignored AllowSharedBookmarks entirely, requiring an exact user match. This method backs Sql::getDefaultSqlQueryForBrowse() (the per-table default browse query feature documented in FAQ 6.22), so a bookmark shared by another user never applied for anyone else, unlike get()/getList() which already honored the setting. getByLabel() now falls back to an empty user the same way the other two do. Extracted the repeated "exact user match, falling back to shared" SQL fragment into a single userMatchCondition() helper used by get(), getList() and getByLabel(). Without it, fixing getByLabel() would have copied a third instance of the same conditional that get()/getList() already duplicated between them. Updated docblocks on all three methods to state the actual contract (shared-across-user and shared-across-database fallback behavior) instead of paraphrasing the method names; getByLabel() now references its real caller so the "why" of the shared-label lookup is not left implicit. Added 8 new tests covering both fixes and their AllowSharedBookmarks-off counterparts, plus the previously untested positive-match path for get(). Updated the two existing tests (BookmarkTest::testGetList and the DbiDummy fixture used by ReplaceControllerTest::testReplace) that had the old SQL text hardcoded, since DbiDummy matches queries by exact string. Signed-off-by: Willian Cavalcante <248522986+cavalcante-willian@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BookmarkRepository::getList()filtered strictly by the requested database,so bookmarks created without a database context (
dbase = '', e.g. saved fromthe Console, which does not require a database to be selected) never appeared
when browsing a specific database's bookmark list, even though the equivalent
shared-user fallback (
user = '') already existed for the user filter.BookmarkRepository::getByLabel()ignored theAllowSharedBookmarkssettingentirely, requiring an exact user match. This method backs
Sql::getDefaultSqlQueryForBrowse(), the per-table default browse queryfeature described in FAQ 6.22, so a bookmark shared by another user never
applied to anyone browsing that table, unlike
get()/getList()whichalready honored the setting consistently.
Both methods now fall back to the shared value the same way
get()alreadydid. The repeated "exact match, falling back to shared" SQL fragment was
extracted into a single
userMatchCondition()helper reused by all threemethods, instead of a third copy of the same conditional.
Commits:
Signed-off-by: Willian Cavalcante 248522986+cavalcante-willian@users.noreply.github.com