Move frontpage selection into the administrator setup workflow - #170
Move frontpage selection into the administrator setup workflow#170snoopdave wants to merge 1 commit into
Conversation
Setup is a read-only bootstrap page. Choosing the initial frontpage weblog is a separate global-administrator action reached by POST (FrontpageSetup); later changes go through global configuration, which resolves the handle through the shared FrontpageSettings service. The service stores the weblog's canonical handle, treats a missing aggregation checkbox as false, writes both properties before a single flush, and clears the site-wide, page and feed caches so the change is visible on this node. save() rejects non-POST requests. Peers pick the change up when their own cache entries expire; Roller has no cross-node invalidation transport and this does not add one. Claude-Session: https://claude.ai/code/session_01A1fhY1E2PCFU6UAPXu2WtV
mraible
left a comment
There was a problem hiding this comment.
Locking the setup save behind global admin is right, but as it stands this breaks the site root for any install without a frontpage weblog and can lock an install out of setup permanently. Details inline; the first two are blockers.
index.jspforwards/tosetup.rolwheneversite.frontpage.weblog.handleis blank, andSetup.execute()now returnsDENIEDfor every non-admin onceuserCount > 0. So a site with users but no frontpage, including a fresh install the moment its first user registers, serves the access-denied tile as its home page to anonymous visitors.- With
users.firstUserAdmin=falsenobody is ever admin, sosetup.rol,frontpageSetup!saveandglobalConfigare all closed and the frontpage can never be chosen. - Deleting the frontpage weblog leaves the stale handle in place (
removeWeblogdoesn't clear it) andSetupnow bounces admins to/instead of offering the chooser. INPUTfromfrontpageSetup!saverendersSetup.jspagainst an action that has none ofuserCount/blogCount/weblogs/bootstrap, so the error page is three empty panels.Setup.jspstill renders the chooser form in bootstrap mode (guard isblogCount > 0, heading isblogCount > 0 && !bootstrap).addErrorfollowed byredirectActionloses the "already configured" message (noMessageStoreinterceptor inrollerStack).GlobalConfigvalidates throughFrontpageSettings.resolveWeblogbut never callsapply(), so the cache invalidation the class promises for both screens only happens on the setup path, andFrontpageSettingsis the firstbusinessclass importing fromui.rendering.
Suggested shape: keep Setup.execute() viewable by anyone (it only shows counts and links, as before), render the chooser form only when isUserIsAdmin() is true, and keep the POST action admin-gated as you have it. That preserves / for visitors and keeps the security win.
| WebloggerFactory.getWeblogger().flush(); | ||
| // Beyond that point this is a site configuration screen. | ||
| if (!isUserIsAdmin()) { | ||
| return DENIED; |
There was a problem hiding this comment.
index.jsp (lines 29-31) forwards / here whenever the frontpage handle is blank, so returning DENIED for non-admins makes the home page an access-denied tile for every visitor of a site that has users but no frontpage yet (checkPermission with a null user throws and isUserIsAdmin() swallows it to false). The page only showed counts and links before; I'd keep execute() open and gate the form in the JSP on isUserIsAdmin(), leaving the POST admin-only.
|
|
||
| WebloggerFactory.getWeblogger().flush(); | ||
| // Beyond that point this is a site configuration screen. | ||
| if (!isUserIsAdmin()) { |
There was a problem hiding this comment.
With users.firstUserAdmin=false (documented in roller.properties) no account ever passes isUserIsAdmin(), and frontpageSetup!save and globalConfig are admin-only too, so the frontpage can never be chosen and / stays broken. The removed setup!save was the only non-admin path; if it goes, the description should say how such installs are expected to finish setup.
|
|
||
| addMessage("frontpageConfig.values.saved"); | ||
| try { | ||
| if (FrontpageSettings.isConfigured()) { |
There was a problem hiding this comment.
JPAWeblogManagerImpl.removeWeblog doesn't clear site.frontpage.weblog.handle, so after the frontpage weblog is deleted isConfigured() is still true and this redirects to /, which forwards to the missing weblog. On master the admin could come back here and pick another one. Either treat a handle that doesn't resolve as unconfigured, or clear the property in removeWeblog.
|
|
||
| } catch (FrontpageSettings.InvalidFrontpageWeblogException ex) { | ||
| addError("frontpageConfig.invalidWeblog"); | ||
| return INPUT; |
There was a problem hiding this comment.
struts.xml maps input for this action to the .Setup tile, but Setup.jsp reads userCount, blogCount, weblogs and bootstrap, none of which exist on FrontpageSetup, so every <s:if> is false and the admin sees the error above three empty panels with no chooser. Either redirect back to setup carrying the message, or give this action the same properties.
| @@ -93,7 +93,8 @@ | |||
|
|
|||
| <s:if test="blogCount > 0"> | |||
There was a problem hiding this comment.
The heading was changed to blogCount > 0 && !bootstrap but this guard wasn't, so in bootstrap mode (weblogs exist, zero users, weblogs is null) anonymous visitors still get the form with an empty select and a Save that POSTs to an admin-only action.
| // losing caller is told the choice is already made. | ||
| if (FrontpageSettings.isConfigured()) { | ||
| addError("frontpageConfig.alreadyConfigured"); | ||
| return "home"; |
There was a problem hiding this comment.
rollerStack has no MessageStore interceptor, so this addError is discarded by the redirectAction; the second admin just lands on the winner's frontpage. The comment above says the loser is told, but they aren't.
| return null; | ||
| } | ||
| return WebloggerFactory.getWeblogger().getWeblogManager() | ||
| .getWeblogByHandle(handle.trim(), Boolean.TRUE); |
There was a problem hiding this comment.
getWeblogByHandle throws WebloggerException("Invalid handle") for anything outside [A-Za-z0-9_], so a POST with frontpageBlog=my-blog takes the generic WebloggerException branch (stack trace at ERROR, "Error saving properties") instead of the invalid-weblog message this method documents. Pre-check the handle or catch that case here.
| if (FrontpageSettings.resolveWeblog(incomingProp) == null) { | ||
| addError("frontpageConfig.invalidWeblog"); | ||
| } else { | ||
| updProp.setValue( incomingProp.trim() ); |
There was a problem hiding this comment.
This validates via resolveWeblog but then stores the raw value itself instead of calling FrontpageSettings.apply(), so changing the frontpage from Global Config skips the cache invalidation the class javadoc promises for "both screens" and keeps serving the old weblog's cached pages and feeds.
| import org.apache.roller.weblogger.WebloggerException; | ||
| import org.apache.roller.weblogger.pojos.RuntimeConfigProperty; | ||
| import org.apache.roller.weblogger.pojos.Weblog; | ||
| import org.apache.roller.weblogger.ui.rendering.util.cache.SiteWideCache; |
There was a problem hiding this comment.
First business → ui.rendering dependency in the tree; SiteWideCache / WeblogPageCache / WeblogFeedCache are rendering singletons that bootstrap caches on getInstance(). The invalidation belongs in the actions (FrontpageSetup / GlobalConfig), with FrontpageSettings just resolving and storing.
| int setupIdx = struts.indexOf("name=\"setup\""); | ||
| assertTrue(setupIdx > 0, "setup action not found in struts.xml"); | ||
| String setupBlock = struts.substring(setupIdx, struts.indexOf("</action>", setupIdx)); | ||
| assertFalse(setupBlock.contains("save"), |
There was a problem hiding this comment.
These assert on source text (contains("save"), method=\"post\"), so an XML comment containing save in the setup block fails the build while a reachable-but-unguarded action passes. requiredGlobalPermissionActions is the only behavioural check here; I'd drop the text ones.
The setup page is reachable without a login so a freshly installed site can be
bootstrapped before any user exists. This change makes that page a read-only
bootstrap view and moves the frontpage-weblog selection into a separate
global-administrator action.
What changed
no weblog lists, counts, or mutating action.
administrator permission, allowed only while no frontpage weblog has been set.
GlobalConfig.service used by both write paths; reject blank, missing, nonexistent, and
disabled weblogs and treat a missing aggregation checkbox as false.
caches, including the site-wide last-modified value.
users.firstUserAdmin=falseuntil a global administrator isprovisioned.
Tests
disabled admin and global-admin callers.
writes; the retired
setup!saveaction no longer resolves.invalidation through both write paths.