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

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

Open
gumaerc opened this issue Apr 8, 2024 · 5 comments
Open

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

gumaerc opened this issue Apr 8, 2024 · 5 comments
Labels
bug Something isn't working product:ocw

Comments

@gumaerc
Copy link
Collaborator

gumaerc commented Apr 8, 2024

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.

@gumaerc gumaerc added bug Something isn't working product:ocw labels Apr 8, 2024
@pdpinch
Copy link
Member

pdpinch commented Apr 8, 2024

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

@HussainTaj-arbisoft
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Collaborator Author

gumaerc commented Apr 9, 2024

@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
Copy link
Contributor

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/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working product:ocw
Projects
None yet
Development

No branches or pull requests

3 participants