On Fri, Feb 6, 2009 at 1:42 PM, Emmanouil Batsis (Manos) <[email protected]> wrote: > Dave wrote: > >> 1) There is a local path built into the patches >> "/Users/manos/lib/roller_4.1" >> I get the message "patch cannot be applied in this context" when I >> attempt to apply them against either the trunk or the 4.0 branch. I >> create my patches with "svn diff > patchfile.txt" and it creates >> relative instead of absolute paths. > >> 2) They are made against the Roller 4.0 code base >> These are new features and as such, should really go into the trunk. > > > Done [1] for the trunk, but couldn't update "Affects Version/s:" in jira. > > Also, I dunno why eclipse insists using absolute paths in the patches. I > modified all these in the new patch and then tried to apply the patch in > roller project's basedir; works OK. > > [1] https://issues.apache.org/roller/browse/ROL-1786
Thank you very much for submitting this patch. I apologize for taking so long to review it. Over the weekend, I took a close look and found that this patch: a) allows any user to create a "named pin" for a weblog entry, a unique string that identifies the weblog entry across the entire site. If one user adds a named pin "foo" to a published weblog entry and then a second user does the same, the "foo" pin will be removed from the first user's entry and added to the second user's entry. The code does not care about named pin collisions except for published entries. b) adds a text field to the weblog entry add and edit pages, right below the title where a user can add a named pin for an entry. c) adds method WeblogEntryManager.getPinnedEntres(String status, Integer max) so that one can query for pinned entries with a specific name. d) adds a method SiteModel.getNamedPinEntryMap() so that named pin entries can be retrieved (see the screenshot attached to the issue for an example use case). It seems to me that this is a cumbersome way to allow a site administrator to pick entries to be featured in specific locations in the front page of a site. A site administrator will have to edit the individual entries that he wishes to feature on the main page, instead of nominating entries from a central UI. The named pin field clutters the entry add and edit page with a new field that is not useful and confusing to most users. It also requires that a new field be added to weblog entry table. I would much prefer to see this feature implemented like so: 1) add a new Global Entry Management page, like the existing Global Comment Management page, to the Server Admin section of the UI. 2) In that new page, an admin user would be able to set and unset the Pinned to Main flag for any entry by setting a check box. 3) Add a new argument to the getWeblogEntriesPinnedToMain() method so that a tag can be specified when querying for pinned entries. Do the same thing for the corresponding method in the SiteModel. With this approach, a site administrator could setup a front-page blog with different sections for "featured" entries and each section would be assigned a tag. Users who wish to have their entries featured would know to use those tags on their entries. Every day or week an admin user would login to Roller to decide which entries should be featured. He would use the Global Entry Management page, filter by each the featured tags and choose the entries to be featured by setting the pinned-to-main checkfox for entries he selected. Each featured section in the front page theme could be setup to show only the most recent 1 or N pinned to main entries in the tag that is assigned to the section. We might want to rename the "pinned-to-main" field to "featured" at least in the UI to make it more clear what the purposes is. That way we would not need to add a new database field, we would not need to add additional UI to the entry add/edit page and the site administrator would not have to edit individual entries to pin them to the main page of the site. Does that make sense? - Dave
