Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Migrate data for media field in repeatable field #33111

Conversation

joomdonation
Copy link
Contributor

@joomdonation joomdonation commented Apr 12, 2021

Pull Request for Issue # .

Summary of Changes

When data for a Repeatable field from Joomla 3 migrated to a Subfields on Joomla 4, the value for media field type is stored in String, not in JSON format as expected, thus it throws some warnings when the field is being displayed (see #32611 (comment) )

This PR attempts to solve that.

Testing Instructions

  1. Install Joomla 3.10.0. You can download the nightly build package here https://developer.joomla.org/nightlies/Joomla_3.10.0-alpha6-dev-Development-Full_Package.zip to install it.

  2. Access to Content -> Fields, create a Repeatable field. This field should contains at least one Media field for testing purpose (see screenshot for example)
    repeatable_field

  3. Create one or two articles. When you add article, navigate to Fields tab, enter data for the repeatable field which you created above. See screenshot for example data
    screenshot

  4. Upgrade your site to latest Joomla 4.0-dev package (generate for this PR). Just download the updated package here https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33111/downloads/42298/Joomla_4.0.0-beta8-dev+pr.33111-Development-Update_Package.zip, then go to Components -> Joomla ! Update, look Upload & Update tab and install that update package.

  5. Check the articles on upgraded site, edit these article, check and make sure data in Fields tab is there.
    field_data

  6. Set the article to Featured so that it is displayed on homepage of the site

  7. Access to the site in the frontend, see the article with custom fields data displayed. Make sure there are no warning. Unfortunately, the image which you select in the field for article could not be displayed because of a bug (is solved in a different PR), so ignore it. Just make sure there are no warnings and the test could be marked as success

Home

@joomdonation
Copy link
Contributor Author

@HLeithner Could you please take a look when you have some free time?

@RickR2H
Copy link
Member

RickR2H commented Apr 23, 2021

I have tested this item ✅ successfully on 9701f71

Test was successful! Thanks for the work!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33111.

@HLeithner
Copy link
Member

@joomdonation thanks looks good but only did a code review

@TLWebdesign
Copy link

I have tested this item ✅ successfully on 9701f71

I tested this during JoomlaDayUSA BF@H successfully on a live website running php 7.3.26 and mysql: 10.4.17-MariaDB. Others were unsuccessful tho.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33111.

@TLWebdesign
Copy link

Following nr.4 in testing instructions the test fail.

When using the custom URL https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33111/downloads/42298/pr_list.xml it works just fine.

@BertaOctech
Copy link

I am writing this just for the record.

The failure was caused because the attributes

?joomla_image_width=710&joomla_image_height=202

where missing in the field value which migrated from J3.10

On comparison between successful and unsuccessful tests we found out the difference was the upgrade package used to upgrade J!3_10 to J!4

The one offered above
https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33111/downloads/42298/Joomla_4.0.0-beta8-dev+pr.33111-Development-Update_Package.zip,
was producing errors on migration.

Using
https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33111/downloads/42298/pr_list.xml
as update server worked correctly


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33111.

@joomdonation
Copy link
Contributor Author

@TLWebdesign @BertaOctech Maybe it is because my testing instructions is outdated.

For the issue with the image mentioned by @BertaOctech , I remember it was fixed in a different PR already

@Quy
Copy link
Contributor

Quy commented Apr 23, 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33111.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 23, 2021
@rdeutz rdeutz added this to the Joomla 4.0 milestone Apr 24, 2021
@rdeutz rdeutz merged commit a5195a0 into joomla:4.0-dev Apr 24, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 24, 2021
@joomdonation joomdonation deleted the fix_data_migration_for_media_field_in_repetable_field branch April 24, 2021 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants