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

Wrong baseline magnolia version on dam 2.4

    Details

    • Type: Bug
    • Status: Accepted
    • Priority: Neutral
    • Resolution: Unresolved
    • Affects Version/s: 2.4
    • Fix Version/s: None
    • Component/s: DAM External App
    • Labels:
      None

      Description

      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.

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Date of First Response: