Merge branch '358909-reintroduce-flag-for-jupyter-notebook-diffing' into 'master'

Reintroduce flag for jupyter notebook diffing

See merge request gitlab-org/gitlab!85079

config/feature_flags/development/ipynb_semantic_diff.yml

+---
+name: ipynb_semantic_diff
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85079
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/358917
+milestone: '15.0'
+type: development
+group: group::code review
+default_enabled: true

doc/user/project/repository/jupyter_notebooks/index.md

@@ -25,8 +25,14 @@ GitLab.
 ## Cleaner diffs
-> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/6589) in GitLab 14.5 [with a flag](../../../../administration/feature_flags.md) named `jupyter_clean_diffs`. Enabled by default.
+> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/6589) in GitLab 14.5 as an [Alpha](../../../../policy/alpha-beta-support.md#alpha-features) release [with a flag](../../../../administration/feature_flags.md) named `jupyter_clean_diffs`. Enabled by default.
 > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75500) in GitLab 14.9. Feature flag `jupyter_clean_diffs` removed.
+> - [Reintroduced toggle](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85079) in GitLab 15.0 [with a flag](../../../../administration/feature_flags.md) named `ipynb_semantic_diff`. Enabled by default.
+
+FLAG:
+On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the feature flag](../../../../administration/feature_flags.md) named `ipynb_semantic_diff`.
+On GitLab.com, this feature is available.
+This feature is ready for production use.
 When commits include changes to Jupyter Notebook files, GitLab:
@@ -37,6 +43,10 @@ Code suggestions are not available on diffs and merge requests for `.ipynb` file
 ![Jupyter Notebook Clean Diff](img/jupyter_notebook_diff_v14_5.png)
+This feature is an [Alpha](../../../../policy/alpha-beta-support.md#alpha-features) release,
+and might lead to performance degradation. On self-managed GitLab, if unexpected issues
+arise, disable the feature.
+
 ## Jupyter Git integration
 Jupyter can be configured as an OAuth application with repository access, acting

lib/gitlab/diff/file.rb

@@ -44,7 +44,13 @@ module Gitlab
         new_blob_lazy
         old_blob_lazy
-        diff.diff = Gitlab::Diff::CustomDiff.preprocess_before_diff(diff.new_path, old_blob_lazy, new_blob_lazy) || diff.diff unless use_renderable_diff?
+        if use_semantic_ipynb_diff? && !use_renderable_diff?
+          diff.diff = Gitlab::Diff::CustomDiff.preprocess_before_diff(diff.new_path, old_blob_lazy, new_blob_lazy) || diff.diff
+        end
+      end
+
+      def use_semantic_ipynb_diff?
+        strong_memoize(:_use_semantic_ipynb_diff) { Feature.enabled?(:ipynb_semantic_diff, repository.project, default_enabled: :yaml) }
       end
       def use_renderable_diff?
@@ -375,7 +381,7 @@ module Gitlab
       end
       def rendered
-        return unless use_renderable_diff? && ipynb? && modified_file? && !too_large?
+        return unless use_semantic_ipynb_diff? && use_renderable_diff? && ipynb? && modified_file? && !too_large?
         strong_memoize(:rendered) { Rendered::Notebook::DiffFile.new(self) }
       end

spec/lib/gitlab/diff/file_spec.rb

@@ -51,6 +51,54 @@ RSpec.describe Gitlab::Diff::File do
     project.commit(branch_name).diffs.diff_files.first
   end
+  describe '#initialize' do
+    let(:commit) { project.commit("532c837") }
+
+    context 'when file is ipynb' do
+      let(:ipynb_semantic_diff) { false }
+      let(:rendered_diffs_viewer) { false }
+
+      before do
+        stub_feature_flags(ipynb_semantic_diff: ipynb_semantic_diff, rendered_diffs_viewer: rendered_diffs_viewer)
+      end
+
+      context 'when ipynb_semantic_diff is off, and rendered_viewer is off' do
+        it 'does not generate notebook diffs' do
+          expect(Gitlab::Diff::CustomDiff).not_to receive(:preprocess_before_diff)
+          expect(diff_file.rendered).to be_nil
+        end
+      end
+
+      context 'when ipynb_semantic_diff is off, and rendered_viewer is on' do
+        let(:rendered_diffs_viewer) { true }
+
+        it 'does not generate rendered diff' do
+          expect(Gitlab::Diff::CustomDiff).not_to receive(:preprocess_before_diff)
+          expect(diff_file.rendered).to be_nil
+        end
+      end
+
+      context 'when ipynb_semantic_diff is on, and rendered_viewer is off' do
+        let(:ipynb_semantic_diff) { true }
+
+        it 'transforms using custom diff CustomDiff' do
+          expect(Gitlab::Diff::CustomDiff).to receive(:preprocess_before_diff).and_call_original
+          expect(diff_file.rendered).to be_nil
+        end
+      end
+
+      context 'when ipynb_semantic_diff is on, and rendered_viewer is on' do
+        let(:ipynb_semantic_diff) { true }
+        let(:rendered_diffs_viewer) { true }
+
+        it 'transforms diff using NotebookDiffFile' do
+          expect(Gitlab::Diff::CustomDiff).not_to receive(:preprocess_before_diff)
+          expect(diff_file.rendered).not_to be_nil
+        end
+      end
+    end
+  end
+
   describe '#has_renderable?' do
     context 'file is ipynb' do
       let(:commit) { project.commit("532c837") }
@@ -104,6 +152,20 @@ RSpec.describe Gitlab::Diff::File do
           expect(diff_file.rendered).to be_nil
         end
       end
+
+      context 'when semantic ipynb is off' do
+        before do
+          stub_feature_flags(ipynb_semantic_diff: false)
+        end
+
+        it 'returns nil' do
+          expect(diff_file).not_to receive(:modified_file?)
+          expect(diff_file).not_to receive(:ipynb?)
+          expect(diff).not_to receive(:too_large?)
+
+          expect(diff_file.rendered).to be_nil
+        end
+      end
     end
   end