Merge branch...

Merge branch '28601-have-gitlab-render-both-latex-and-images-correctly-in-jupyter-notebooks-2' into 'master'

Rendering images with relative path on Jupyter Notebooks

See merge request gitlab-org/gitlab!69075

app/assets/javascripts/blob/notebook/index.js

@@ -6,6 +6,9 @@ export default () => {
 
   return new Vue({
     el,
+    provide: {
+      relativeRawPath: el.dataset.relativeRawPath,
+    },
     render(createElement) {
       return createElement(NotebookViewer, {
         props: {

app/assets/javascripts/notebook/cells/markdown.vue

@@ -94,7 +94,16 @@ renderer.image = function image(href, title, text) {
   const attachmentHeader = `attachment:`; // eslint-disable-line @gitlab/require-i18n-strings
 
   if (!this.attachments || !href.startsWith(attachmentHeader)) {
-    return this.originalImage(href, title, text);
+    let relativeHref = href;
+
+    // eslint-disable-next-line @gitlab/require-i18n-strings
+    if (!(href.startsWith('http') || href.startsWith('data:'))) {
+      // These are images within the repo. This will only work if the image
+      // is relative to the path where the file is located
+      relativeHref = this.relativeRawPath + href;
+    }
+
+    return this.originalImage(relativeHref, title, text);
   }
 
   let img = ``;
@@ -129,6 +138,7 @@ export default {
   components: {
     prompt: Prompt,
   },
+  inject: ['relativeRawPath'],
   props: {
     cell: {
       type: Object,
@@ -138,6 +148,7 @@ export default {
   computed: {
     markdown() {
       renderer.attachments = this.cell.attachments;
+      renderer.relativeRawPath = this.relativeRawPath;
 
       return sanitize(marked(this.cell.source.join('').replace(/\\/g, '\\\\')), markdownConfig);
     },

app/helpers/blob_helper.rb

@@ -183,6 +183,10 @@ module BlobHelper
     blob_raw_url(**kwargs, only_path: true)
   end
 
+  def parent_dir_raw_path
+    blob_raw_path.rpartition("/").first + "/"
+  end
+
   # SVGs can contain malicious JavaScript; only include whitelisted
   # elements and attributes. Note that this whitelist is by no means complete
   # and may omit some elements.

app/presenters/snippet_blob_presenter.rb

@@ -17,6 +17,10 @@ class SnippetBlobPresenter < BlobPresenter
     snippet_blob_raw_route
   end
 
+  def raw_directory
+    raw_path.rpartition("/").first + "/"
+  end
+
   def raw_plain_data
     blob.data unless blob.binary?
   end
@@ -33,7 +37,7 @@ class SnippetBlobPresenter < BlobPresenter
 
   def render_rich_partial
     renderer.render("projects/blob/viewers/_#{blob.rich_viewer.partial_name}",
-                    locals: { viewer: blob.rich_viewer, blob: blob, blob_raw_path: raw_path, blob_raw_url: raw_url },
+                    locals: { viewer: blob.rich_viewer, blob: blob, blob_raw_path: raw_path, blob_raw_url: raw_url, parent_dir_raw_path: raw_directory },
                     layout: false)
   end
 

app/views/projects/blob/viewers/_notebook.html.haml

-.file-content#js-notebook-viewer{ data: { endpoint: blob_raw_path } }
+.file-content#js-notebook-viewer{ data: { endpoint: blob_raw_path, relative_raw_path: parent_dir_raw_path } }

spec/frontend/blob/notebook/notebook_viever_spec.js

@@ -11,6 +11,7 @@ describe('iPython notebook renderer', () => {
   let mock;
 
   const endpoint = 'test';
+  const relativeRawPath = '';
   const mockNotebook = {
     cells: [
       {
@@ -27,7 +28,7 @@ describe('iPython notebook renderer', () => {
   };
 
   const mountComponent = () => {
-    wrapper = shallowMount(component, { propsData: { endpoint } });
+    wrapper = shallowMount(component, { propsData: { endpoint, relativeRawPath } });
   };
 
   const findLoading = () => wrapper.find(GlLoadingIcon);

spec/frontend/notebook/cells/markdown_spec.js

+import { mount } from '@vue/test-utils';
 import katex from 'katex';
 import Vue from 'vue';
 import MarkdownComponent from '~/notebook/cells/markdown.vue';
@@ -6,6 +7,28 @@ const Component = Vue.extend(MarkdownComponent);
 
 window.katex = katex;
 
+function buildCellComponent(cell, relativePath = '') {
+  return mount(Component, {
+    propsData: {
+      cell,
+    },
+    provide: {
+      relativeRawPath: relativePath,
+    },
+  }).vm;
+}
+
+function buildMarkdownComponent(markdownContent, relativePath = '') {
+  return buildCellComponent(
+    {
+      cell_type: 'markdown',
+      metadata: {},
+      source: markdownContent,
+    },
+    relativePath,
+  );
+}
+
 describe('Markdown component', () => {
   let vm;
   let cell;
@@ -17,12 +40,7 @@ describe('Markdown component', () => {
     // eslint-disable-next-line prefer-destructuring
     cell = json.cells[1];
 
-    vm = new Component({
-      propsData: {
-        cell,
-      },
-    });
-    vm.$mount();
+    vm = buildCellComponent(cell);
 
     return vm.$nextTick();
   });
@@ -61,17 +79,36 @@ describe('Markdown component', () => {
     expect(findLink().getAttribute('data-type')).toBe(null);
   });
 
+  describe('When parsing images', () => {
+    it.each([
+      [
+        'for relative images in root folder, it does',
+        '![](local_image.png)\n',
+        'src="/raw/local_image',
+      ],
+      [
+        'for relative images in child folders, it does',
+        '![](data/local_image.png)\n',
+        'src="/raw/data',
+      ],
+      ["for embedded images, it doesn't", '![](data:image/jpeg;base64)\n', 'src="data:'],
+      ["for images urls, it doesn't", '![](http://image.png)\n', 'src="http:'],
+    ])('%s', async ([testMd, mustContain]) => {
+      vm = buildMarkdownComponent([testMd], '/raw/');
+
+      await vm.$nextTick();
+
+      expect(vm.$el.innerHTML).toContain(mustContain);
+    });
+  });
+
   describe('tables', () => {
     beforeEach(() => {
       json = getJSONFixture('blob/notebook/markdown-table.json');
     });
 
     it('renders images and text', () => {
-      vm = new Component({
-        propsData: {
-          cell: json.cells[0],
-        },
-      }).$mount();
+      vm = buildCellComponent(json.cells[0]);
 
       return vm.$nextTick().then(() => {
         const images = vm.$el.querySelectorAll('img');
@@ -102,48 +139,28 @@ describe('Markdown component', () => {
     });
 
     it('renders multi-line katex', async () => {
-      vm = new Component({
-        propsData: {
-          cell: json.cells[0],
-        },
-      }).$mount();
+      vm = buildCellComponent(json.cells[0]);
 
       await vm.$nextTick();
       expect(vm.$el.querySelector('.katex')).not.toBeNull();
     });
 
     it('renders inline katex', async () => {
-      vm = new Component({
-        propsData: {
-          cell: json.cells[1],
-        },
-      }).$mount();
+      vm = buildCellComponent(json.cells[1]);
 
       await vm.$nextTick();
       expect(vm.$el.querySelector('p:first-child .katex')).not.toBeNull();
     });
 
     it('renders multiple inline katex', async () => {
-      vm = new Component({
-        propsData: {
-          cell: json.cells[1],
-        },
-      }).$mount();
+      vm = buildCellComponent(json.cells[1]);
 
       await vm.$nextTick();
       expect(vm.$el.querySelectorAll('p:nth-child(2) .katex')).toHaveLength(4);
     });
 
     it('output cell in case of katex error', async () => {
-      vm = new Component({
-        propsData: {
-          cell: {
-            cell_type: 'markdown',
-            metadata: {},
-            source: ['Some invalid $a & b$ inline formula $b & c$\n', '\n'],
-          },
-        },
-      }).$mount();
+      vm = buildMarkdownComponent(['Some invalid $a & b$ inline formula $b & c$\n', '\n']);
 
       await vm.$nextTick();
       // expect one paragraph with no katex formula in it
@@ -152,15 +169,10 @@ describe('Markdown component', () => {
     });
 
     it('output cell and render remaining formula in case of katex error', async () => {
-      vm = new Component({
-        propsData: {
-          cell: {
-            cell_type: 'markdown',
-            metadata: {},
-            source: ['An invalid $a & b$ inline formula and a vaild one $b = c$\n', '\n'],
-          },
-        },
-      }).$mount();
+      vm = buildMarkdownComponent([
+        'An invalid $a & b$ inline formula and a vaild one $b = c$\n',
+        '\n',
+      ]);
 
       await vm.$nextTick();
       // expect one paragraph with no katex formula in it
@@ -169,15 +181,7 @@ describe('Markdown component', () => {
     });
 
     it('renders math formula in list object', async () => {
-      vm = new Component({
-        propsData: {
-          cell: {
-            cell_type: 'markdown',
-            metadata: {},
-            source: ["- list with inline $a=2$ inline formula $a' + b = c$\n", '\n'],
-          },
-        },
-      }).$mount();
+      vm = buildMarkdownComponent(["- list with inline $a=2$ inline formula $a' + b = c$\n", '\n']);
 
       await vm.$nextTick();
       // expect one list with a katex formula in it
@@ -186,15 +190,7 @@ describe('Markdown component', () => {
     });
 
     it("renders math formula with tick ' in it", async () => {
-      vm = new Component({
-        propsData: {
-          cell: {
-            cell_type: 'markdown',
-            metadata: {},
-            source: ["- list with inline $a=2$ inline formula $a' + b = c$\n", '\n'],
-          },
-        },
-      }).$mount();
+      vm = buildMarkdownComponent(["- list with inline $a=2$ inline formula $a' + b = c$\n", '\n']);
 
       await vm.$nextTick();
       // expect one list with a katex formula in it
@@ -203,15 +199,7 @@ describe('Markdown component', () => {
     });
 
     it('renders math formula with less-than-operator < in it', async () => {
-      vm = new Component({
-        propsData: {
-          cell: {
-            cell_type: 'markdown',
-            metadata: {},
-            source: ['- list with inline $a=2$ inline formula $a + b < c$\n', '\n'],
-          },
-        },
-      }).$mount();
+      vm = buildMarkdownComponent(['- list with inline $a=2$ inline formula $a + b < c$\n', '\n']);
 
       await vm.$nextTick();
       // expect one list with a katex formula in it
@@ -220,15 +208,7 @@ describe('Markdown component', () => {
     });
 
     it('renders math formula with greater-than-operator > in it', async () => {
-      vm = new Component({
-        propsData: {
-          cell: {
-            cell_type: 'markdown',
-            metadata: {},
-            source: ['- list with inline $a=2$ inline formula $a + b > c$\n', '\n'],
-          },
-        },
-      }).$mount();
+      vm = buildMarkdownComponent(['- list with inline $a=2$ inline formula $a + b > c$\n', '\n']);
 
       await vm.$nextTick();
       // expect one list with a katex formula in it

spec/frontend/notebook/index_spec.js

+import { mount } from '@vue/test-utils';
 import Vue from 'vue';
 import Notebook from '~/notebook/index.vue';
 
@@ -13,14 +14,16 @@ describe('Notebook component', () => {
     jsonWithWorksheet = getJSONFixture('blob/notebook/worksheets.json');
   });
 
+  function buildComponent(notebook) {
+    return mount(Component, {
+      propsData: { notebook, codeCssClass: 'js-code-class' },
+      provide: { relativeRawPath: '' },
+    }).vm;
+  }
+
   describe('without JSON', () => {
     beforeEach((done) => {
-      vm = new Component({
-        propsData: {
-          notebook: {},
-        },
-      });
-      vm.$mount();
+      vm = buildComponent({});
 
       setImmediate(() => {
         done();
@@ -34,13 +37,7 @@ describe('Notebook component', () => {
 
   describe('with JSON', () => {
     beforeEach((done) => {
-      vm = new Component({
-        propsData: {
-          notebook: json,
-          codeCssClass: 'js-code-class',
-        },
-      });
-      vm.$mount();
+      vm = buildComponent(json);
 
       setImmediate(() => {
         done();
@@ -66,13 +63,7 @@ describe('Notebook component', () => {
 
   describe('with worksheets', () => {
     beforeEach((done) => {
-      vm = new Component({
-        propsData: {
-          notebook: jsonWithWorksheet,
-          codeCssClass: 'js-code-class',
-        },
-      });
-      vm.$mount();
+      vm = buildComponent(jsonWithWorksheet);
 
       setImmediate(() => {
         done();

spec/helpers/blob_helper_spec.rb

@@ -92,6 +92,30 @@ RSpec.describe BlobHelper do
     end
   end
 
+  describe "#relative_raw_path" do
+    include FakeBlobHelpers
+
+    let_it_be(:project) { create(:project) }
+
+    before do
+      assign(:project, project)
+    end
+
+    [
+      %w[/file.md /-/raw/main/],
+      %w[/test/file.md /-/raw/main/test/],
+      %w[/another/test/file.md /-/raw/main/another/test/]
+    ].each do |file_path, expected_path|
+      it "pointing from '#{file_path}' to '#{expected_path}'" do
+        blob = fake_blob(path: file_path)
+        assign(:blob, blob)
+        assign(:id, "main#{blob.path}")
+        assign(:path, blob.path)
+
+        expect(helper.parent_dir_raw_path).to eq "/#{project.full_path}#{expected_path}"
+      end
+    end
+  end
   context 'viewer related' do
     include FakeBlobHelpers
 

spec/presenters/snippet_blob_presenter_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe SnippetBlobPresenter do
 
   describe '#rich_data' do
     let(:data_endpoint_url) { "/-/snippets/#{snippet.id}/raw/#{branch}/#{file}" }
+    let(:data_raw_dir) { "/-/snippets/#{snippet.id}/raw/#{branch}/" }
 
     before do
       allow_next_instance_of(described_class) do |instance|
@@ -45,7 +46,7 @@ RSpec.describe SnippetBlobPresenter do
         let(:file) { 'test.ipynb' }
 
         it 'returns rich notebook content' do
-          expect(subject.strip).to eq %Q(<div class="file-content" data-endpoint="#{data_endpoint_url}" id="js-notebook-viewer"></div>)
+          expect(subject.strip).to eq %Q(<div class="file-content" data-endpoint="#{data_endpoint_url}" data-relative-raw-path="#{data_raw_dir}" id="js-notebook-viewer"></div>)
         end
       end