snoopdave commented on code in PR #246:
URL: https://github.com/apache/roller/pull/246#discussion_r4569391350
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/Bookmarks.java:
##########
@@ -68,202 +75,222 @@ public Bookmarks() {
this.pageTitle = "bookmarksForm.rootTitle";
}
+ /**
+ * Loads the requested blogroll. Without one, opens the default blogroll if
+ * it has links, otherwise the first blogroll that has links, so the page
+ * doesn't open on an empty list while the links are elsewhere.
+ */
@Override
public void myPrepare() {
try {
BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- if (!StringUtils.isEmpty(getFolderId())) {
- setFolder(bmgr.getFolderById(getActionWeblog(),
getFolderId()));
- } else {
- setFolder(bmgr.getDefaultFolder(getActionWeblog()));
- if (getFolder() != null) {
- setFolderId(getFolder().getId());
- }
+ List<WeblogBookmarkFolder> folders = new
ArrayList<>(bmgr.getAllFolders(getActionWeblog()));
+ folders.sort(Comparator
+ .comparing((WeblogBookmarkFolder f) ->
!DEFAULT_FOLDER.equals(f.getName()))
+ .thenComparing(f ->
StringUtils.defaultString(f.getName()), String.CASE_INSENSITIVE_ORDER));
+ allFolders = folders;
+
+ WeblogBookmarkFolder requested = StringUtils.isEmpty(getFolderId())
+ ? null : bmgr.getFolderById(getActionWeblog(),
getFolderId());
+ if (requested == null) {
+ requested = folders.stream()
+ .filter(f -> !f.getBookmarks().isEmpty())
+ .findFirst()
+ .orElse(folders.isEmpty() ? null : folders.get(0));
}
+ setFolder(requested);
} catch (WebloggerException ex) {
log.error("Error looking up folder", ex);
}
}
/**
- * Present the bookmarks available in the folder specified by the request.
+ * Shows the blogroll.
*/
+ @SkipValidation
@Override
public String execute() {
-
- // build list of folders that the user can navigate to
- List<WeblogBookmarkFolder> newFolders = new ArrayList<>();
-
- try {
- // Build list of all folders, except for current one
- BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- List<WeblogBookmarkFolder> folders =
bmgr.getAllFolders(getActionWeblog());
- for (WeblogBookmarkFolder fd : folders) {
- if (!fd.getId().equals(getFolderId())) {
- newFolders.add(fd);
- }
- }
-
- } catch (WebloggerException ex) {
- log.error("Error building folders list", ex);
- addError("Error building folders list");
- }
-
- if (!newFolders.isEmpty()) {
- setAllFolders(newFolders);
- }
-
return LIST;
}
/**
- * Delete bookmarks.
+ * Deletes the selected links.
*/
public String delete() {
-
- BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
-
+ List<WeblogBookmark> links = selectedLinks();
+ if (links == null) {
+ addError("bookmarksForm.error.notFound");
+ return execute();
+ }
try {
- WeblogBookmark bookmark;
- String bookmarks[] = getSelectedBookmarks();
- if (null != bookmarks && bookmarks.length > 0) {
- if (log.isDebugEnabled()) {
- log.debug("Processing delete of " + bookmarks.length
- + " bookmarks.");
- }
- for (int j = 0; j < bookmarks.length; j++) {
- if (log.isDebugEnabled()) {
- log.debug("Deleting bookmark - " + bookmarks[j]);
- }
- bookmark = bmgr.getBookmark(getActionWeblog(),
bookmarks[j]);
- if (bookmark != null) {
- bmgr.removeBookmark(bookmark);
- }
-
- }
+ BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
+ for (WeblogBookmark link : links) {
+ folder.getBookmarks().remove(link);
+ bmgr.removeBookmark(link);
}
-
- // flush changes
WebloggerFactory.getWeblogger().flush();
-
- // notify caches
CacheManager.invalidate(getActionWeblog());
-
+ addMessage(links.size() == 1 ? "bookmarksForm.deletedOne" :
"bookmarksForm.deletedMany",
+ String.valueOf(links.size()));
} catch (WebloggerException ex) {
- log.error("Error doing bookmark deletes", ex);
- addError("Error doing bookmark deletes");
+ log.error("Error deleting links", ex);
+ addError("generic.error.check.logs");
}
-
return execute();
}
+ /**
+ * Deletes the blogroll being viewed and its links, then shows the default
blogroll.
+ */
public String deleteFolder() {
-
+ if (folder == null) {
+ addError("folderForm.notFound");
+ return execute();
+ }
+ if (isDefaultFolder()) {
+ addError("bookmarksForm.error.deleteDefault");
+ return execute();
+ }
try {
BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- WeblogBookmarkFolder fd = bmgr.getFolderById(getActionWeblog(),
getFolderId());
-
- if (fd != null) {
-
- if ( "default".equals( fd.getName() ) ) {
- addError("Cannot delete default bookmark");
- return execute();
- }
- bmgr.removeFolder(fd);
-
- // flush changes
- WebloggerFactory.getWeblogger().flush();
-
- // notify caches
- CacheManager.invalidate(getActionWeblog());
-
- // re-route to default folder
- setFolder(bmgr.getDefaultFolder(getActionWeblog()));
- setFolderId(getFolder().getId());
- }
+ String name = folder.getName();
+ bmgr.removeFolder(folder);
+ WebloggerFactory.getWeblogger().flush();
+ CacheManager.invalidate(getActionWeblog());
+ addMessage("bookmarksForm.folderDeleted", name);
+ allFolders = new ArrayList<>(allFolders);
+ allFolders.remove(folder);
+ setFolder(bmgr.getDefaultFolder(getActionWeblog()));
} catch (WebloggerException ex) {
log.error("Error deleting folder", ex);
+ addError("generic.error.check.logs");
}
return execute();
}
@SkipValidation
public String folderCreated() {
- // action from FolderEdit upon creation of a new folder, to display
- // a success message prior to showing the new folder.
- addMessage("folderForm.created");
+ // reached by a redirect from FolderEdit after it creates a folder, and
+ // from BookmarksImport after it imports links into a new one
+ if (folder != null) {
+ addMessage("folderForm.created", folder.getName());
+ }
return execute();
}
/**
- * View the contents of another bookmark folder.
+ * Moves the selected links to the end of another blogroll.
*/
- public String view() {
-
+ public String move() {
try {
BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- if (!StringUtils.isEmpty(viewFolderId)) {
- setFolder(bmgr.getFolderById(getActionWeblog(), viewFolderId));
- setFolderId(viewFolderId);
+ WeblogBookmarkFolder target =
StringUtils.isEmpty(getTargetFolderId())
+ ? null : bmgr.getFolderById(getActionWeblog(),
getTargetFolderId());
+ List<WeblogBookmark> links = selectedLinks();
+ if (target == null || links == null || target.equals(folder)) {
+ addError("bookmarksForm.error.move");
+ return execute();
}
- } catch (WebloggerException ex) {
- log.error("Error looking up folder", ex);
+ for (WeblogBookmark link : links) {
+ folder.getBookmarks().remove(link);
+ target.addBookmark(link);
+ link.setFolder(target);
+ // recalculated by saveBookmark: after the target's last link
+ link.setPriority(null);
+ bmgr.saveBookmark(link);
+ }
+ WebloggerFactory.getWeblogger().flush();
+ CacheManager.invalidate(getActionWeblog());
+ addMessage(links.size() == 1 ? "bookmarksForm.movedOne" :
"bookmarksForm.movedMany",
+ List.of(String.valueOf(links.size()), target.getName()));
+ } catch (WebloggerException e) {
+ log.error("Error moving links", e);
+ addError("bookmarksForm.error.move");
}
return execute();
}
/**
- * Move bookmarks to a new folder.
+ * Saves a new order for the links of the blogroll. The posted ids must be
+ * exactly the blogroll's links.
*/
- public String move() {
-
+ public String reorder() {
+ if (folder == null) {
+ addError("folderForm.notFound");
+ return execute();
+ }
+ List<WeblogBookmark> links = folder.getBookmarks();
+ Set<String> ids = new HashSet<>(bookmarkIds);
+ if (ids.size() != bookmarkIds.size() || ids.size() != links.size()
+ || links.stream().anyMatch(link ->
!ids.contains(link.getId()))) {
+ addError("bookmarksForm.error.reorder");
+ return execute();
+ }
try {
BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
-
- if (log.isDebugEnabled()) {
- log.debug("Moving bookmarks to folder - " +
getTargetFolderId());
+ for (WeblogBookmark link : links) {
+ link.setPriority(bookmarkIds.indexOf(link.getId()));
Review Comment:
🤖Claude: fixed in 7d910483d: `reorder()` builds an id-to-position map, sets
the priorities on the managed links and saves the weblog once. A test checks
that `saveBookmark` is not called.
##########
app/src/main/webapp/WEB-INF/jsps/tiles/messages.jsp:
##########
@@ -19,8 +19,9 @@
<script>
$(document).ready(function () {
- // Remove the alert box after 10 seconds
- $(".alert").delay(10000).slideUp(200, function() {
+ // Remove the action message boxes below after 10 seconds. Only these:
other
+ // alerts on the page, such as error boxes in dialogs, must stay.
+ $("#messages, #errors").delay(10000).slideUp(200, function() {
Review Comment:
🤖Claude: fixed in 7d910483d: same change as on #245: auto-hide skips alerts
inside a `.modal` and applies everywhere else.
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/Bookmarks.java:
##########
@@ -68,202 +75,222 @@ public Bookmarks() {
this.pageTitle = "bookmarksForm.rootTitle";
}
+ /**
+ * Loads the requested blogroll. Without one, opens the default blogroll if
+ * it has links, otherwise the first blogroll that has links, so the page
+ * doesn't open on an empty list while the links are elsewhere.
+ */
@Override
public void myPrepare() {
try {
BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- if (!StringUtils.isEmpty(getFolderId())) {
- setFolder(bmgr.getFolderById(getActionWeblog(),
getFolderId()));
- } else {
- setFolder(bmgr.getDefaultFolder(getActionWeblog()));
- if (getFolder() != null) {
- setFolderId(getFolder().getId());
- }
+ List<WeblogBookmarkFolder> folders = new
ArrayList<>(bmgr.getAllFolders(getActionWeblog()));
+ folders.sort(Comparator
+ .comparing((WeblogBookmarkFolder f) ->
!DEFAULT_FOLDER.equals(f.getName()))
+ .thenComparing(f ->
StringUtils.defaultString(f.getName()), String.CASE_INSENSITIVE_ORDER));
+ allFolders = folders;
+
+ WeblogBookmarkFolder requested = StringUtils.isEmpty(getFolderId())
+ ? null : bmgr.getFolderById(getActionWeblog(),
getFolderId());
+ if (requested == null) {
+ requested = folders.stream()
+ .filter(f -> !f.getBookmarks().isEmpty())
+ .findFirst()
+ .orElse(folders.isEmpty() ? null : folders.get(0));
}
+ setFolder(requested);
} catch (WebloggerException ex) {
log.error("Error looking up folder", ex);
}
}
/**
- * Present the bookmarks available in the folder specified by the request.
+ * Shows the blogroll.
*/
+ @SkipValidation
@Override
public String execute() {
-
- // build list of folders that the user can navigate to
- List<WeblogBookmarkFolder> newFolders = new ArrayList<>();
-
- try {
- // Build list of all folders, except for current one
- BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- List<WeblogBookmarkFolder> folders =
bmgr.getAllFolders(getActionWeblog());
- for (WeblogBookmarkFolder fd : folders) {
- if (!fd.getId().equals(getFolderId())) {
- newFolders.add(fd);
- }
- }
-
- } catch (WebloggerException ex) {
- log.error("Error building folders list", ex);
- addError("Error building folders list");
- }
-
- if (!newFolders.isEmpty()) {
- setAllFolders(newFolders);
- }
-
return LIST;
}
/**
- * Delete bookmarks.
+ * Deletes the selected links.
*/
public String delete() {
-
- BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
-
+ List<WeblogBookmark> links = selectedLinks();
+ if (links == null) {
+ addError("bookmarksForm.error.notFound");
+ return execute();
+ }
try {
- WeblogBookmark bookmark;
- String bookmarks[] = getSelectedBookmarks();
- if (null != bookmarks && bookmarks.length > 0) {
- if (log.isDebugEnabled()) {
- log.debug("Processing delete of " + bookmarks.length
- + " bookmarks.");
- }
- for (int j = 0; j < bookmarks.length; j++) {
- if (log.isDebugEnabled()) {
- log.debug("Deleting bookmark - " + bookmarks[j]);
- }
- bookmark = bmgr.getBookmark(getActionWeblog(),
bookmarks[j]);
- if (bookmark != null) {
- bmgr.removeBookmark(bookmark);
- }
-
- }
+ BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
+ for (WeblogBookmark link : links) {
+ folder.getBookmarks().remove(link);
+ bmgr.removeBookmark(link);
}
-
- // flush changes
WebloggerFactory.getWeblogger().flush();
-
- // notify caches
CacheManager.invalidate(getActionWeblog());
-
+ addMessage(links.size() == 1 ? "bookmarksForm.deletedOne" :
"bookmarksForm.deletedMany",
+ String.valueOf(links.size()));
} catch (WebloggerException ex) {
- log.error("Error doing bookmark deletes", ex);
- addError("Error doing bookmark deletes");
+ log.error("Error deleting links", ex);
+ addError("generic.error.check.logs");
}
-
return execute();
}
+ /**
+ * Deletes the blogroll being viewed and its links, then shows the default
blogroll.
+ */
public String deleteFolder() {
-
+ if (folder == null) {
+ addError("folderForm.notFound");
+ return execute();
+ }
+ if (isDefaultFolder()) {
+ addError("bookmarksForm.error.deleteDefault");
+ return execute();
+ }
try {
BookmarkManager bmgr =
WebloggerFactory.getWeblogger().getBookmarkManager();
- WeblogBookmarkFolder fd = bmgr.getFolderById(getActionWeblog(),
getFolderId());
-
- if (fd != null) {
-
- if ( "default".equals( fd.getName() ) ) {
- addError("Cannot delete default bookmark");
- return execute();
- }
- bmgr.removeFolder(fd);
-
- // flush changes
- WebloggerFactory.getWeblogger().flush();
-
- // notify caches
- CacheManager.invalidate(getActionWeblog());
-
- // re-route to default folder
- setFolder(bmgr.getDefaultFolder(getActionWeblog()));
- setFolderId(getFolder().getId());
- }
+ String name = folder.getName();
+ bmgr.removeFolder(folder);
+ WebloggerFactory.getWeblogger().flush();
+ CacheManager.invalidate(getActionWeblog());
+ addMessage("bookmarksForm.folderDeleted", name);
+ allFolders = new ArrayList<>(allFolders);
+ allFolders.remove(folder);
+ setFolder(bmgr.getDefaultFolder(getActionWeblog()));
} catch (WebloggerException ex) {
log.error("Error deleting folder", ex);
+ addError("generic.error.check.logs");
}
return execute();
}
@SkipValidation
public String folderCreated() {
- // action from FolderEdit upon creation of a new folder, to display
- // a success message prior to showing the new folder.
- addMessage("folderForm.created");
+ // reached by a redirect from FolderEdit after it creates a folder, and
+ // from BookmarksImport after it imports links into a new one
+ if (folder != null) {
+ addMessage("folderForm.created", folder.getName());
Review Comment:
🤖Claude: fixed in 7d910483d: `folderCreated` shows the message only when the
request named a blogroll and it exists. A test covers the import case with no
id.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
