Renders images on the repository of .ipynb files

Images that were not embedded into Jupyter Notebooks were not being
properly rendered because the path would point to the blob url, not the
raw URL.

Changelog: changed
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/28601
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/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