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

fix: setAssetToStaticUrl regex matcher #497

Merged
merged 1 commit into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/editors/sharedComponents/TinyMceWidget/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,9 @@ export const setAssetToStaticUrl = ({ editorValue, lmsEndpointUrl }) => {
const assetSrcs = typeof content === 'string' ? content.split(/(src="|src="|href="|href=&quot)/g) : [];
assetSrcs.filter(src => src.startsWith('/asset')).forEach(src => {
let nameFromEditorSrc;
if (src.match(/\/assets\/.+\/asset-v1:\S+[+]\S+[@]\S+[+]\S+\//)?.length >= 1) {
if (src.match(/\/asset-v1:\S+[+]\S+[@]\S+[+]\S+\/\w/)?.length >= 1) {
const assetBlockName = src.substring(0, src.search(/("|")/));
const dividedSrc = assetBlockName.split(/\/assets\/.+\/asset-v1:\S+[+]\S+[@]\S+[+]\S+\//);
const dividedSrc = assetBlockName.split(/\/asset-v1:\S+[+]\S+[@]\S+[+]\S+\//);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not match the regex above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because I want to catch all the cases were this appears. If you look at the description for the PR, the first and third bullet show the two ways that asset url can be format. The original regex only caught the first version, not the second and was causing the url tow be incorrectly converted.

[, nameFromEditorSrc] = dividedSrc;
} else {
const assetBlockName = src.substring(src.indexOf('@') + 1, src.search(/("|")/));
Expand Down
9 changes: 5 additions & 4 deletions src/editors/sharedComponents/TinyMceWidget/hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const mockEditorContentHtml = `
</img>
</p>
`;
const baseAssetUrl = 'asset-v1:org+test+run+type@asset+block';

const mockImagesRef = { current: [mockImage] };

Expand Down Expand Up @@ -182,17 +183,17 @@ describe('TinyMceEditor hooks', () => {
});

describe('replaceStaticWithAsset', () => {
const initialContent = '<img src="/static/soMEImagEURl1.jpeg"/><a href="/assets/v1/some-key/test.pdf">test</a><img src="/asset-v1:org+test+run+type@asset+block@correct.png" />';
const initialContent = `<img src="/static/soMEImagEURl1.jpeg"/><a href="/assets/v1/${baseAssetUrl}/test.pdf">test</a><img src="/${baseAssetUrl}@correct.png" />`;
const learningContextId = 'course-v1:org+test+run';
const lmsEndpointUrl = 'sOmEvaLue.cOm';
it('returns updated src for text editor to update content', () => {
const expected = '<img src="/asset-v1:org+test+run+type@asset+block@soMEImagEURl1.jpeg"/><a href="/asset-v1:org+test+run+type@asset+block@test.pdf">test</a><img src="/asset-v1:org+test+run+type@asset+block@correct.png" />';
const expected = `<img src="/${baseAssetUrl}@soMEImagEURl1.jpeg"/><a href="/${baseAssetUrl}@test.pdf">test</a><img src="/${baseAssetUrl}@correct.png" />`;
const actual = module.replaceStaticWithAsset({ initialContent, learningContextId });
expect(actual).toEqual(expected);
});
it('returns updated src with absolute url for expandable editor to update content', () => {
const editorType = 'expandable';
const expected = `<img src="${lmsEndpointUrl}/asset-v1:org+test+run+type@asset+block@soMEImagEURl1.jpeg"/><a href="${lmsEndpointUrl}/asset-v1:org+test+run+type@asset+block@test.pdf">test</a><img src="${lmsEndpointUrl}/asset-v1:org+test+run+type@asset+block@correct.png" />`;
const expected = `<img src="${lmsEndpointUrl}/${baseAssetUrl}@soMEImagEURl1.jpeg"/><a href="${lmsEndpointUrl}/${baseAssetUrl}@test.pdf">test</a><img src="${lmsEndpointUrl}/${baseAssetUrl}@correct.png" />`;
const actual = module.replaceStaticWithAsset({
initialContent,
editorType,
Expand All @@ -209,7 +210,7 @@ describe('TinyMceEditor hooks', () => {
});
describe('setAssetToStaticUrl', () => {
it('returns content with updated img links', () => {
const editorValue = '<img src="/asset@/soME_ImagE_URl1"/> <a href="/asset@soMEImagEURl">testing link</a>';
const editorValue = `<img src="/${baseAssetUrl}/soME_ImagE_URl1"/> <a href="/${baseAssetUrl}@soMEImagEURl">testing link</a>`;
const lmsEndpointUrl = 'sOmEvaLue.cOm';
const content = module.setAssetToStaticUrl({ editorValue, lmsEndpointUrl });
expect(content).toEqual('<img src="/static/soME_ImagE_URl1"/> <a href="/static/soMEImagEURl">testing link</a>');
Expand Down
Loading