mraible commented on code in PR #247:
URL: https://github.com/apache/roller/pull/247#discussion_r4559376403
##########
app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java:
##########
@@ -110,6 +114,18 @@ public void removeWeblog(Weblog weblog) throws
WebloggerException {
// remove contents first, then remove weblog
this.removeWeblogContents(weblog);
+
+ // Clear frontpage weblog if set
+ if (WebloggerRuntimeConfig.isFrontPageWeblog(weblog.getHandle())) {
Review Comment:
`isFrontPageWeblog()` does `frontPageHandle.equals(weblogHandle)` with no
null check, and `WebloggerRuntimeConfig.getProperty` returns null when the row
is missing, the value is null, or the lookup fails. With this change every
weblog delete depends on that property being non-null. I'm also wondering about
Oracle, which stores an empty string as NULL, so the `""` written below might
come back as null there. I haven't tested that.
`weblog.getHandle().equals(FrontpageSettings.getConfiguredHandle())` would
be null-safe.
##########
app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java:
##########
@@ -110,6 +114,18 @@ public void removeWeblog(Weblog weblog) throws
WebloggerException {
// remove contents first, then remove weblog
this.removeWeblogContents(weblog);
+
+ // Clear frontpage weblog if set
+ if (WebloggerRuntimeConfig.isFrontPageWeblog(weblog.getHandle())) {
+
+ PropertiesManager mgr =
WebloggerFactory.getWeblogger().getPropertiesManager();
+
+ RuntimeConfigProperty handleProp =
mgr.getProperty(FrontpageSettings.HANDLE_PROPERTY);
+
+ mgr.compareAndSetProperty(FrontpageSettings.HANDLE_PROPERTY,
handleProp.getValue(), "");
+
+ }
Review Comment:
This fixes deleting the frontpage weblog, but `index.jsp` still forwards to
the frontpage weblog whenever the handle is non-blank. A site that already has
a dangling handle, or whose frontpage weblog gets disabled, still gets the 404.
If `index.jsp` used `FrontpageSettings.isConfigured()` (which checks the weblog
exists and is enabled), that would cover delete, disable and existing installs.
It could be instead of this or as well as it.
##########
app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAWeblogManagerImpl.java:
##########
@@ -110,6 +114,18 @@ public void removeWeblog(Weblog weblog) throws
WebloggerException {
// remove contents first, then remove weblog
this.removeWeblogContents(weblog);
+
+ // Clear frontpage weblog if set
+ if (WebloggerRuntimeConfig.isFrontPageWeblog(weblog.getHandle())) {
+
+ PropertiesManager mgr =
WebloggerFactory.getWeblogger().getPropertiesManager();
+
+ RuntimeConfigProperty handleProp =
mgr.getProperty(FrontpageSettings.HANDLE_PROPERTY);
+
+ mgr.compareAndSetProperty(FrontpageSettings.HANDLE_PROPERTY,
handleProp.getValue(), "");
Review Comment:
Two things here:
- The expected value is the one just read from the property, so the compare
can't fail in a useful way. If the frontpage were switched to another weblog
between the check above and this read, this would clear the new weblog's
setting. Passing `weblog.getHandle()` as the expected value makes the compare
meaningful and removes the extra `getProperty` call (and the NPE if
`handleProp` is null).
- Only the handle is cleared. `site.frontpage.weblog.aggregated` keeps its
old value, and `FrontpageSettings` says the two move together. If an aggregated
frontpage weblog is deleted, the next weblog chosen in Global Config starts
with the aggregated box already checked.
A small `FrontpageSettings.clearIfFrontpage(weblog)` that resets both would
keep all the writers of these properties in one place.
##########
CHANGES.md:
##########
@@ -1,5 +1,11 @@
# Apache Roller — Changes
+## 6.1.8
+
+### Bug fixes
+- **Check and clear site.frontpage.weblog.handle** If weblog being deleted is
front page
+ clear property so Welcome to Roller page is shown.
Review Comment:
Could this describe what a user sees? Something like: "Deleting the
frontpage weblog no longer leaves the site home page returning a 404."
--
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]
