mraible commented on code in PR #247:
URL: https://github.com/apache/roller/pull/247#discussion_r4566798089
##########
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:
Sorry, I wasn't clear. I agree the weblog's handle won't be null. The one
that can be is the property: `isFrontPageWeblog` does
`frontPageHandle.equals(weblogHandle)`, and
`getProperty("site.frontpage.weblog.handle")` returns null when the row is
missing or the lookup fails. Then deleting any weblog throws an NPE. Flipping
the comparison avoids it:
`weblog.getHandle().equals(WebloggerRuntimeConfig.getProperty(FrontpageSettings.HANDLE_PROPERTY))`.
##########
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:
That makes sense, keeping the admin's choice is a fair reason to leave it.
Thanks for switching the expected value to the weblog's handle.
##########
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:
Agreed, this is a real improvement for the delete case. The case it doesn't
reach is a handle that's already dangling (an existing install, or a frontpage
weblog that gets disabled): `index.jsp` still forwards because the handle is
non-blank. Using `FrontpageSettings.isConfigured()` there would cover it. Happy
for that to be a separate PR.
--
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]
