Change Donation Modal body text to aRichTextField

[mtdenton]

Stakeholder: Michael Whitney
Original request: https://mozilla-hub.atlassian.net/browse/MOFOTP-23

We would like to be able to add formatting (bold, line breaks, etc.) to the donation modal snippets after you sign petitions, so it’s not just a big block of plain text.

Needs groomed, but seems like we should change body from TextField to RichTextField. We should also put a reasonable character limit here. We should also find out if header should remain a CharField or convert that as well.

Open questions:
Current data should also be considered. How will the data be migrated? Can we cleanly migrate a text field to a rich text field?

┆Issue is synchronized with this Jira Task
┆Attachments: image (7).png | image-20240510-214746.png

[jhonatan-lopes]

Current data should also be considered. How will the data be migrated? Can we cleanly migrate a text field to a rich text field?

[simont-cr]

@jhonatan-lopes Added this as a P1 as we might want to go back to the stakeholder and let them know if we're either doing this or not. I would say that if the process of migrating is going to be difficult and might cause unintended issues, we might be better off leaving this as it is for the time being. The original deadline for this request was Q1 2024.

[jhonatan-lopes]

We can do the data migrations, it just needs to be well thought before proceeding: https://docs.wagtail.org/en/stable/advanced_topics/streamfield_migrations.html#streamfield-migrations

[simont-cr]

Let's discuss this in our next sprint planning. Thank you!

[mmmavis]

@jhonatan-lopes The field name body is probably misleading as we are not talking about the body field of a page here 😅 The donate modal is a snippet that has a field called body of type TextField. I'm assuming changing the field type to RichTextField would be simply updating body = models.TextField( to body = models.RichTextField( and generating the migration file? Is there anything we should worry about for the existing data? For this case it's unlikely that existing data would contain HTML tags in them but I'm still curious generally if we need to run anything to sanitize the data.

[jhonatan-lopes]

@mmmavis yep, I'm aware that it is a snippet 😬

We should definitely start by doing what you said: converting body = models.TextField to body = wagtail_fields.RichTextField and then creating a migration. The migration that will be created will alter the field to be a RichTextField, something like:

    operations = [
        migrations.AlterField(
            model_name="donationmodal",
            name="body",
            field=wagtail.fields.RichTextField(
                default="...",
                help_text="Donation text",
            ),
        ),
    ]

What I'm not sure is what comes next. I'm not 100% certain if the AlterField migration knows how to convert between a TextField and a RichTextField automatically so that the data makes sense as a rich text. So, we need to run the migration and verify. If everything looks good with the default migration, then hoooray 🎉 we got nothing to worry about.

If not, we will need to figure out how to handle that data so that it is "rich text valid". Does that make sense?

[mmmavis]

@jhonatan-lopes Yup! I'll get a copy of prod data to test it locally.

[mmmavis]

@jhonatan-lopes Not sure if this is the right way of comparing the before and after values (certainly not the most efficient way I think 😅 ) ...

What I did is to export data in the DonationModal table into json (both before and after the migration), and compare the body values of the before/after data to see if any pair didn't match. In the Python script I used == to do the string value comparison. It turned out all "after" body values were identical to their "before" counterpart. I also went into localhost CMS and picked a random donation modal snippet to check its rendering result, which looked the same as what we have on prod.

Is this the kind of check you had in mind? If not, please let me know how I should proceed. Thanks!

[jhonatan-lopes]

If the rendering still works, then that's good enough for me :thumbs-up:

Thanks for checking that

[mmmavis]

@kristinashu @mtdenton

What elements do we wanna allow in the rich text field? I'm thinking just italic, bold, and link. For reference, the section we are talking about here is just the "body" of the donation modal (e.g., Thank you for signing the petition to call...)

[kristinashu]

@mmmavis I'd recommend you ask Christian since he has a better sense of what the copy would be, I'm not sure what he has in mind.

[mmmavis]

Sounds good. Will do!

[data-sync-user]

➤ Mavis Ou commented:

Current update:

I reached out to Christian on Slack yesterday. I will update the ticket and file PR once I hear back from him.

[data-sync-user]

➤ Mavis Ou commented:

Update:

Christian and team would like to have every feature in the RichTextField ( https://docs.wagtail.org/en/stable/advanced_topics/customisation/page_editing_interface.html#limiting-features-in-a-rich-text-field ) enabled.

!image (7).png|width=1624,height=824,alt="image (7).png"!

[data-sync-user]

➤ Mavis Ou commented:

Simon Acosta Torres I just tested this on staging, however, on staging the HTML formatted string doesn’t get rendered as HTML. This is different than what we tested it on the PR stage. I’m gonna dive into it more. If this can’t be fixed before the next deploy then we’ll have to pull out the code changes from main

!image-20240510-214746.png|width=695,height=383,alt="image-20240510-214746.png"!

[data-sync-user]

➤ Mavis Ou commented:

Fix is pretty straightforward. Will file a PR now.

[data-sync-user]

➤ Simon Acosta Torres commented:

Mavis Ou Thanks for taking a look and even more, for finding a fix for this. This is one of the last few things remaining from stakeholders, so it will be amazing to be able to say its done 🙂

[data-sync-user]

➤ Mavis Ou commented:

Fix is under review at the moment: #12322 ( https://github.com/MozillaFoundation/foundation.mozilla.org/pull/12322|smart-link )

[data-sync-user]

➤ Mavis Ou commented:

PR merged and tested on staging. This is ready for prod push!