Bug 109947 - The formbuilder content types are installed in the wrong section
The formbuilder content types are installed in the wrong section
Product: Red Hat Web Application Framework
Classification: Retired
Component: other (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Vadim Nasardinov
Jon Orris
: 110025 (view as bug list)
Depends On:
Blocks: 109665
  Show dependency treegraph
Reported: 2003-11-13 05:01 EST by Daniel Berrange
Modified: 2007-04-18 12:59 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2003-12-03 15:06:52 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
transcript of an IRC chat with Dan (7.23 KB, text/plain)
2003-12-01 17:36 EST, Vadim Nasardinov
no flags Details

  None (edit)
Description Daniel Berrange 2003-11-13 05:01:18 EST
Description of problem:
The form builder content types (FormItem & FormSectionItem) are being
installed into /content instead of /forms. Since we can't neccessarily
guarentee that either /content or /forms exists for any given install
we should make the location configurable. The reason for having two
content sections is because when in use, interactive forms are
typically managed in a different way & by different people to the
normal text based content. I suggest either two properties in
AbstractContentTypeLoader, with default values to match previous 6.0 /
5.2 config, ie:


This makes the cotent types install in the same configuration as
previous releases of CMS & also enables the admin to trivially
relocate them to be in alternative sections if required.

Or alternatively let each individual content type provide a config
property for its section and add an

   abstract String getContentSection();

method to AbstractContentTypeLoader which all types must implement.
Or then again, maybe provide a default impl of this methodwhich
returns /content & let the FormItem types override it.

Whichever method is chosen in the end though, there must be a config
property to override the defualt of /content & /forms since neither
section may exist.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
1. Install the ccm-cms-types-formitem package
2. Run the loader
3. Go to content type
Actual results:
FormItem is in /content section

Expected results:
FormItem is in /forms section

Additional info:
Comment 1 Daniel Berrange 2003-11-14 04:55:54 EST
*** Bug 110025 has been marked as a duplicate of this bug. ***
Comment 2 Vadim Nasardinov 2003-11-24 16:10:02 EST
Fixed on the trunk in change 38262.

The fix is implemented as follows.

Quoting from change 38262:

  * Load FormSectionItem and FormItem into the "/forms/" section.

  * For any content type X, any content section S such that X is not in
    {FormSectionItem, FormItem} and S is not "/forms/", load X into S.

The literal string "forms" is hardcoded into two files (with two
additional files having indirect knowledge of it):


The following two files have indirect knowledge of the literal string
"forms" (via the symbolic constant


The class AbstractContentTypeLoader has acquired one additional

  protected boolean isLoadableInto(ContentSection section);

Note that the method is not abstract.
Comment 3 Daniel Berrange 2003-11-25 04:34:22 EST
This is not QA_READY since it doesn't address the following point:

> Whichever method is chosen in the end though, there must be a config
> property to override the defualt of /content & /forms since neither
> section may exist.

Comment 4 Vadim Nasardinov 2003-11-25 11:26:51 EST
The ticket was marked QA_READY because change 38262 eliminates the
out-of-box regression from 6.0.

As for addressing the more general issue, I have a strong feeling we
need to expend a little more mental energy on this.

> I suggest either two properties in AbstractContentTypeLoader, with
> default values to match previous 6.0 / 5.2 config, ie:
>    com.arsdigita.cms.contenttypes.content_section=/content
>    com.arsdigita.cms.contenttypes.form_section=/forms

One of these values seems redundant.  The distinction seems to be that
FormSectionItem (FSI) and FormItem (FI) should go into the "forms"
section.  Any other content type should go into all other content
sections.  Therefore, it would seem sufficient to specify either the
"form_section" parameter or the "non_form_section" parameter, but not

> Or alternatively let each individual content type provide a config
> property for its section and add an
>    abstract String getContentSection();

This makes the assumption that each content type only wants to be
installed in a single section.  Are we willing to hard-code this

The current implementation is that AbstractContentTypeLoader (ACTL)
has the method

  boolean isLoadableInto(ContentSection);

so that the createTypes method can do this:

  DataCollection sections = ssn.retrieve(ContentSection.TYPE);

  while (sections.next()) {
      ContentSection section = (ContentSection)
      if ( !isLoadableInto(section) ) { continue; }

      // do stuff

The default implementation is

    protected boolean isLoadableInto(ContentSection section) {
        return  !"forms".equals(section.getName());

My question is, how configurable do you want this to be?  One option
would be something like this:

    protected boolean isLoadableInto(ContentSection section) {
        String specialSection = getParam("form_section");
        return  !specialSection.equals(section.getName());

That would be the default implementation.  The value of the
"form_section" parameter would be configurable.  FILoader and
FSILoader can override ACTL's implementation like so:

    protected boolean isLoadableInto(ContentSection section) {
        return !super.isLoadableInto(section);

You can go one step further and say that ACTL defines this method:

public abstract class ACTL {
   public static interface CSDecider {
       boolean isLoadableInto(ContentSection)

   public CSDecider getCSDecider() {
      CSDecider decider = getParameter("decider_class_name");
      return decider;

and you can make the name of the particular implementation of this
interface configurable.  This would help you get around the rather
arbitrary, APLAWS-centric assumption that there is this single special
content section that is very different from all other sections.

Comment 5 Jon Orris 2003-12-01 11:03:51 EST
Dan & Vadim came to a consensus on IRC. Vadim plans to fix for Dec 5th.
If possible, can this conversation thread be posted to the ticket?
Comment 6 Vadim Nasardinov 2003-12-01 11:17:58 EST
    justin,danpb_uml_monkey: have you had the time to look at
    https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=109947 ?
    vadim: i think we're getting into slight overkill here
    with having pluggable APIs for deciding what section to install into
    could we simply make do with the following:
    have a config property on AbstractContnetTypeLoader which is the name of
    the default content section to install types in
    optionally have a config property per subclass which provides a list of
    content sections to put that particular type in - this overrides the
    default setting
    so in simple case, an admin could set
    and have it apply to all content types
    if a particular type needed to go elsewhere they could set
    or even
    if it needed to go in many places
    danpb_uml_monkey: I still don't have a clear picture in my head.  You just
    give too many options.
    I'm not sure users wouldn't prefer, in general, to load non-special types
    in all sections and then opt-out of them via the section UI.  in other
    words, I think this level of config may go little used
    but this is a little OT: vadim's claim seems to be that the form types are
    special, and need special handling
    so, ACTL has a parameter that specifies the (single) default section.  And
    individual type loaders have a (list) property that overrides the default
    my claim is, based on what has been said, FI and FSI are the only two
    types that need special handling.
    vadim: yes, the (list) per type could be a (single) too if we like
    vadim: we've also got a couple of other form based types in the APLWS code
    do they go into the same "forms" section that FI and FSI go into?
    yeah --- ashah has changed the topic to:
    this question seems clearer to me if you say that the special thing is the
    "forms" section
    it's a prepackaged section with prepackaged types
    likewise 'content' section is a prepackaged section with a bunch of
    prepackaged types
    every other section is, logically, not part of the distribution but part
    of the deployment
    if we had sitemap UI to create sections on the section we wouldn't need to
    pre-populate anything really
    justin: perhaps we're thinking about this entirely the wrong way
    maybe a single map parameter containing
    a content section ->
    list of types mapping is whats needed ?
    I'm trying to optimize for the most common case.  If someone doesn't like
    the default setting, they have to override it.  If the controlling
    parameter is "default_content_section", then you have to override for each
    of your 35 content types.  If the controlling parameter is "form_section",
    then you have to override for only 3 or 4 special content types.
    vadim: you could always change 'default_content_section' param itself
    thus affecting every single type at once
    yes, you're right
    I think a section ->
    type mapping makes some sense.  trouble is, I think it gets us into the db
    vs config storage problem.  anyway, we shouldn't try to solve that now, I
    think we need to think expediently
    or, rather, "local optimum"
    oh yeah
    ok, so what's the local optimum?
      List getDefaultSections();
    where getDefaultSections() reads the configurable parameter value?
    and also:
      List getTypeSpecificSections();
    or some such?
    if this return non-null, then getDefaultSections() is ignored?
    vadim: i figure that if getDefaultSections returned null or empty list
    then the general default section would be used
    ok, slow down a bit.  There is a "general section" and "default sections"?

    'List getDefaultSections()' is something that looks at a property for a
    particular contnet type
    if that is not set to anything
    then it falls back to value of cms.types.default_section (or similar)
    I'm headed to lunch.  I'll read the results when I get back
    the return value of getDefaultSections() is a list of strings?  or a list
    of content sections?
    doesn't really matter
    the type of the "default_section" parameter is a String?  or a list of
    a single String i reckon
    awrighty then
Comment 7 Vadim Nasardinov 2003-12-01 17:36:16 EST
Created attachment 96270 [details]
transcript of an IRC chat with Dan

I tried posting this as a comment on the ticket, but it didn't stick
for some reason.  (Although I did get an email notification.) I'm
guessing the comment size was too big.	So there.
Comment 8 Vadim Nasardinov 2003-12-01 18:11:10 EST
This is partially addressed in 38397.  Here's how the current setup

All content type loaders currently extend AbstractContentTypeLoader
(ACTL). The latter has the following method.

  protected List getContentSections();

It returns an empty list.  If a custom content type loader chooses not
to override this method to return a non-empty list, then ACTL will
load the content type into content section specified by
ContentSection.getConfig().getDefaultContentSection().  The latter
returns the value of the
parameter, defaulting to "content".

If the custom content type loader overrides the getContentSections()
method, then the value of the "default_content_section" parameter is
ignored.  The content type is loaded only into those sections that are
contained in the list returned by getContentSections().

There are currently two custom content type loaders that override the
method getContentSections().  These are FormSectionItemLoader
(hereinafter FSILoader) and FormItemLoader (FILoader).  Their
implementations are _nearly_identical_.  The only part that differs is
the parameter name.  Makes me wonder if it would be useful to provide
a mechanism for overriding a default parameter value on a case-by-case
basis.  The specific use case I have in mind is the following.

  Current setup:

    FSILoader and FILoader have nearly identical implementations of
    getContentSections().  The former returns the value of
    com.arsdigita.cms.formbuilder.FormSectionItem.sections and the
    latter com.arsdigita.cms.formbuilder.FormItem.sections.

  Desirable setup:

    Factor out the duplicate implementations out of FSILoader and
    FILoader and provide a single implementation in ACTL.  The ACTL
    implementation would return the value of
    To customize this value for specific content types, we would
    create, say, FSILoader_parameter.properties that looks like so:

    formbuilder.FormSectionItem.sections = overrides
    formbuilder.FormSectionItem.sections.default = forms

    In other words, all I want to do is override the value of a
    parameter on a per content-type basis.  It seems a little
    gratuitous to have to copy and paste lines of code in order to do
    this.  My proposal seems to be out in the left field.  Perhaps
    someone can think of a better solution.
Anyhow, the current solution is likely temporary because the current
setup is somewhat fragile.  Sections are currently created by
SectionInitializer backed by enterprise.init.  The literal strings
"content" and "forms" are hardcoded in

Note You need to log in before you can comment on or make changes to this bug.