Uploaded image for project: 'Magnolia DAM Module'
  1. Magnolia DAM Module
  2. MGNLDAM-760

Wrong baseline magnolia version on dam 2.4

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Obsolete
    • Icon: Neutral Neutral
    • None
    • 2.4
    • DAM External App
    • None

      For some reason dam 2.4 doesn't depend on 5.7 yet.

      • release/2.3 is the 5.6-compatible branch
      • there's a compilation issues because of the decoupling from Vaadin's deprecated Converter API
        • see info.magnolia.ui.form.field.converter.FieldValueConverter (Magnolia replacement)
        • see MGNLUI-4363

      Then the update turned out to be more complicated, primarily on AssetLinkFieldFactoryTest:

      • #linkField_SetFieldPropagation starts to fail because the value propagation to the inner text-field *is* effectively broken in this scenario (textField never has a datasource of his own anymore, since MGNLUI-4368)
        • analog test in UI's LinkFieldFactoryTest passes by cheating, because the last assertion was made completely bogus (yay, testing Vaadin's #setValue ftw)

      Turns out AssetLinkFieldFactory is an obsolete/duplicate of an old version of UI's LinkFieldFactory. We should:

      • Deprecate and phase out usage of AssetLinkField, since it does not have any value on top of the stock LinkField
      • Make AssetLinkFieldFactory extend LinkFieldFactory instead of copy-pasta
      • Keep merely the #createChooseDialogCallback method, that's the one feeding the choose-dialog selected value back as a DAM ItemKey to the LinkField.

      When doing the above, AssetLinkFieldFactoryTest#simpleLinkFieldUuidTest starts to fail too:

      • because the stock LinkFieldFactory passes the identifierToPathConverter via LinkField#setTextFieldConverter
      • the AssetLinkFieldFactory does not handle identifierToPathConverter at all —nor contentPreview, fwiw.
        • confirmed by commenting out this behavior from super implementation, test passes again...
      • surprising, given documentation examples include the identifierToPathConverter in configuration.
        • see [https://documentation.magnolia-cms.com/display/DOCS57/External+DAM+app+module|External DAM app module]
        • see [https://git.magnolia-cms.com/projects/DOCUMENTATION/repos/cumulus-templates-examples/browse/dialogs/components/cumulusSimpleImage.yaml#9|cumulusSimpleImage.yaml]

      We should check dam-external-app (via cumulus or S3 module) on 5.7, and figure out what it used to do when identifierToPathConverter is configured.

        Acceptance criteria

              Unassigned Unassigned
              mgeljic Mikaël Geljić
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved:

                  Bug DoR
                  Task DoD