mitodl / ocw-hugo-themes

A Hugo theme for building OCW websites
BSD 3-Clause "New" or "Revised" License
4 stars 4 forks source link

resource_link shortcode throws an error if you pass in a title #1387

Open gumaerc opened 4 months ago

gumaerc commented 4 months ago

Expected Behavior

If I have a call to the resource_link shortcode in my content, i.e. ({{% resource_link "f6e70b41-9390-494c-a569-f825b3faa01e" "Inspiration" %}}), a link (a tag) should be generated with the text passed in the second argument.

Current Behavior

An error is thrown:

Error: error building site: assemble: "/tmp/build/ed66b15e/site-content-git/content/pages/image-galleries/jewelry-gallery.md:1:1": got positional parameter 'f6e70b41-9390-494c-a569-f825b3faa01e'. Cannot mix named and positional parameters

Steps to Reproduce

  1. Clone an existing content repo or make a new one
  2. Find a page or resource under the content folder and note its UUID
  3. On another page, link to it using the resource_link shortcode, passing the UUID in as the first argument and a title in as the second

Possible Solution

Pull out the expected arguments from .Params and set them explicitly when calling external_resource_link, rather than using merge

Additional Details

There is a method on shortcodes called .IsNamedParams (https://gohugo.io/templates/shortcode-templates/#isnamedparams) that can check if the shortcode has had named params passed to it. This may be helpful. I did my testing with 21m.715-fall-2009 after running the link_to_external_resource and nav_item_to_external_resource management commands in ocw-studio.

pdpinch commented 4 months ago

Can we add a test case for this to the test course?

HussainTaj-arbisoft commented 4 months ago

The Hugo error is a bit misleading. This error is caused by unescaped ".

For example, the following will throw the error

{{< image-gallery-item text="... {{% resource_link "7a3ee310-cc48-457b-90ca-0f72c4851b1b" "Inspiration" %}} ..." >}}

and the following syntax is correct

{{< image-gallery-item text="... {{% resource_link \"7a3ee310-cc48-457b-90ca-0f72c4851b1b\" \"Inspiration\" %}} ..." >}}

However, the image gallery isn't rendered correctly. I updated the image-gallery-item shortcode, but it didn't show the special markup, which leads me to believe that we can't use HTML for image gallery parameters.

In short, this problem appears when an external link is replaced (with an external resource) within a shortcode argument.

As a next step, I'll try to figure out the number of such cases. I'll need to test more legacy courses.

HussainTaj-arbisoft commented 4 months ago

I've found 8 instances of this issue in 3 courses on RC data. All of them are external links in the image gallery's image captions.

Here's a complete list: out.csv

@gumaerc did you come across any other issues?

Since this is a small number, we could exclude these courses from the migration, or manually fix these 3 pages after the migration. I say this because handling this case in the migration command's code will make it complicated (and I don't see an obvious way to handle it in the current implementation).

gumaerc commented 4 months ago

@HussainTaj-arbisoft Thanks for looking into this. Today I went through and manually corrected the pages listed in your CSV in my local database. After pushing up the fixed content, I was able to get all the way through the mass build so we shouldn't have any more trouble after this is fixed. The image gallery is a legacy feature and these are all legacy courses. That combined with the fact that there isn't much to fix means we should just manually correct this data in RC / production. However, we should still test another scenario. Say after we apply this fix, someone opens these pages in ocw-studio and applies some changes, then hits save. Will the escaped quotes be un-escaped?

HussainTaj-arbisoft commented 4 months ago

Say after we apply this fix, someone opens these pages in ocw-studio and applies some changes, then hits save. Will the escaped quotes be un-escaped?

I tested this out and the content doesn't break. That said, the manual fix for this issue is to not use external resources. This is because our image-gallery-item does not use RenderString. If we were to use RenderString, HTML inside the image's caption breaks the layout. So we're better off not modifying the content.

I took another look at these galleries and it turns out the current markdown links are not rendered either. They just appear as plain text. https://live-qa.ocw.mit.edu/courses/21m-715-the-craft-of-costume-design-fall-2009/pages/image-galleries/jewelry-gallery/