snoopdave commented on code in PR #246:
URL: https://github.com/apache/roller/pull/246#discussion_r4569221476
##########
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 Issue: **Important:** this tile is shared by every page that shows
messages, and #245 makes the same edit. Narrowing the auto-hide from `.alert`
to `#messages, #errors` changes other pages: any other `.alert` box that used
to fade after 10 seconds (ThemeEdit, TemplateEdit, MediaFileAdd, WeblogConfig)
now stays. Check those pages, or keep the old selector and exclude dialog
alerts with `.not()`. Land the change in one PR only, so the two don't conflict.
##########
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 Issue: **Important:** `folderCreated` now runs after an import too,
but `BookmarksImport` sets `folderId` to null when it cannot find the folder it
just wrote. The redirect then opens the first blogroll with links, and this
line says "created" for that unrelated blogroll. Show the message only when the
requested `folderId` matched a real folder (check `getFolderId()` against
`folder.getId()`), and add a test for the null case.
--
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]
