Uploaded image for project: 'Magnolia Standard Templating Kit (closed)'
  1. Magnolia Standard Templating Kit (closed)
  2. MGNLSTK-713

Re-fix MGNLSTK-651 (navigation configuration)

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Obsolete
    • Icon: Major Major
    • None
    • 1.3.5
    • None
    • None

      Just installed stk 1.3.5 and the navigation nodes contain openAll (set to "allOpen") and allOpen (correctly set to true or false).
      Looks like the bootstrap files haven't been properly updated when fixing MGNLSTK-651. (they contain both properties with the above mentioned values)

      The STK version handler does its job on updates, but only checks for the horizontal navigation properties - it should also check for the vertical one, as the user might have manually fixed one and not the other.

      IMHO,

                     .addTask(new PropertyExistsDelegateTask("OpenAll", "Checks if openAll property exists in stk horizontal navigation definition and starts the update task if so", ContentRepository.CONFIG, "/modules/standard-templating-kit/config/site/templates/prototype/navigation/horizontal", "openAll", new UpdateSitesDefinitionsFor1_3_5())));
      

      should be something along the lines of

                     .addTask(new FixOpenAllNavigationProperties());
      

      i.e:

      1. no need to first check if the property exists before delegating: do the check in the custom task (besides, unless the user fixed it manually, the "wrong" property will be there)
      2. give the task a meaningful name ("what it does", not "when it's applied")

      Additionally, we probably should re-apply this update in the next releases, since anyone who updated from 1.x to 1.3.5 (with Magnolia <=4.3.8), or installed 1.3.5 directly (with whatever Magnolia version) will still have a broken configuration.

        Acceptance criteria

              pbaerfuss Philipp Bärfuss
              gjoseph Magnolia International
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

                Created:
                Updated:
                Resolved: