From bad84b1f8f97edfae2d341c37f85da1d31072308 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Fri, 14 Jun 2024 20:20:07 +0800
Subject: [PATCH 01/18] Implement LiquidCachedAssigns

---
 .../concerns/liquid_enabled_resource.rb       |  2 +-
 app/services/liquid_assigns_service.rb        | 51 +---------------
 app/services/liquid_cached_assigns.rb         | 59 +++++++++++++++++++
 3 files changed, 63 insertions(+), 49 deletions(-)
 create mode 100644 app/services/liquid_cached_assigns.rb

diff --git a/app/controllers/concerns/liquid_enabled_resource.rb b/app/controllers/concerns/liquid_enabled_resource.rb
index 1dac5829e..04e94046f 100644
--- a/app/controllers/concerns/liquid_enabled_resource.rb
+++ b/app/controllers/concerns/liquid_enabled_resource.rb
@@ -35,6 +35,6 @@ def project_assigns
     project = Project.find(params[:project_id])
     authorize! :use, project
 
-    LiquidAssignsService.new(project: project, text: params[:text]).assigns
+    LiquidAssignsService.new(project: project).assigns
   end
 end
diff --git a/app/services/liquid_assigns_service.rb b/app/services/liquid_assigns_service.rb
index b781c77b6..0260060f3 100644
--- a/app/services/liquid_assigns_service.rb
+++ b/app/services/liquid_assigns_service.rb
@@ -1,61 +1,16 @@
 class LiquidAssignsService
-  AVAILABLE_PROJECT_ASSIGNS = %w{ evidences issues nodes notes tags }.freeze
+  attr_accessor :project
 
-  attr_accessor :project, :text
-
-  def initialize(project:, text: nil)
+  def initialize(project:)
     @project = project
-    @text = text
   end
 
   def assigns
-    result = project_assigns
-    result.merge!(assigns_pro) if defined?(Dradis::Pro)
-    result
+    LiquidCachedAssigns.new(project: project)
   end
 
   private
 
   def assigns_pro
   end
-
-  # This method uses Liquid::VariableLookup to find all liquid variables from
-  # a given text. We use the list to know which project assign we need.
-  def assigns_from_content
-    return AVAILABLE_PROJECT_ASSIGNS if text.nil?
-
-    variable_lookup = Liquid::VariableLookup.parse(text)
-    return (variable_lookup.lookups & AVAILABLE_PROJECT_ASSIGNS)
-  end
-
-  def cached_drops(records, record_type)
-    return [] if records.empty?
-
-    cache_key = "liquid-project-#{project.id}-#{record_type.pluralize}:#{records.maximum(:updated_at).to_i}-#{records.count}"
-    drop_class = "#{record_type.camelize}Drop".constantize
-
-    Rails.cache.fetch(cache_key) do
-      records.map { |record| drop_class.new(record) }
-    end
-  end
-
-  def project_assigns
-    project_assigns = { 'project' => ProjectDrop.new(project) }
-
-    assigns_from_content.each do |record_type|
-      records =
-        case record_type
-        when 'evidences'
-          project.evidence
-        when 'nodes'
-          project.nodes.user_nodes
-        else
-          project.send(record_type.to_sym)
-        end
-
-      project_assigns.merge!(record_type => cached_drops(records, record_type.singularize))
-    end
-
-    project_assigns
-  end
 end
diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
new file mode 100644
index 000000000..cba8cbe8e
--- /dev/null
+++ b/app/services/liquid_cached_assigns.rb
@@ -0,0 +1,59 @@
+class LiquidCachedAssigns < Hash
+  AVAILABLE_PROJECT_ASSIGNS = %w{ evidences issues nodes notes project tags }.freeze
+
+  attr_accessor :assigns, :project
+
+  def initialize(project:)
+    @assigns = { 'project' => ProjectDrop.new(@project) }
+    @project = project
+  end
+
+  def [](record_type)
+    return unless AVAILABLE_PROJECT_ASSIGNS.include?(record_type)
+
+    if assigns[record_type]
+      assigns[record_type]
+    else
+      assigns[record_type] = cached_drops(record_type)
+    end
+  end
+
+  def inspect
+    assigns.inspect
+  end
+
+  def key?(msg)
+    AVAILABLE_PROJECT_ASSIGNS.include?(msg.to_s)
+  end
+
+  def respond_to?(msg)
+    return true if key?(msg)
+    super(msg)
+  end
+
+  private
+
+  def cached_drops(record_type)
+    records = project_records(record_type)
+
+    return [] if records.empty?
+
+    cache_key = ActiveSupport::Cache.expand_cache_key([project.id, records], 'liquid')
+    drop_class = "#{record_type.singularize.camelize}Drop".constantize
+
+    Rails.cache.fetch(cache_key) do
+      records.map { |record| drop_class.new(record) }
+    end
+  end
+
+  def project_records(record_type)
+    case record_type
+    when 'evidences'
+      project.evidence
+    when 'nodes'
+      project.nodes.user_nodes
+    else
+      project.send(record_type.to_sym)
+    end
+  end
+end

From 43eabd7dad727d2d30552cdfd1eff9cd9cc1584a Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Mon, 17 Jun 2024 14:47:19 +0200
Subject: [PATCH 02/18] prevent multiple eventListener binds on tab change

---
 app/assets/javascripts/shared/behaviors.js | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/shared/behaviors.js b/app/assets/javascripts/shared/behaviors.js
index cd6ac4f75..f2d3267ce 100644
--- a/app/assets/javascripts/shared/behaviors.js
+++ b/app/assets/javascripts/shared/behaviors.js
@@ -81,11 +81,13 @@
     }
 
     // Update address bar with current tab param
-    $('[data-bs-toggle~=tab]').on('shown.bs.tab', function (e) {
-      let currentTab = $(e.target).attr('href').substring(1);
-      searchParams.set('tab', currentTab);
-      history.pushState(null, null, `?${searchParams.toString()}`);
-    });
+    $('[data-bs-toggle~=tab]')
+      .unbind()
+      .on('shown.bs.tab', function (e) {
+        let currentTab = $(e.target).attr('href').substring(1);
+        searchParams.set('tab', currentTab);
+        history.pushState(null, null, `?${searchParams.toString()}`);
+      });
   }
 
   document.addEventListener('turbolinks:load', function () {

From 63c6e82de67871f622b1e64724e658f3f7d0e339 Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Mon, 17 Jun 2024 14:49:07 +0200
Subject: [PATCH 03/18] navigate to tabbed pages using browser back button

---
 app/assets/javascripts/shared/behaviors.js | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/app/assets/javascripts/shared/behaviors.js b/app/assets/javascripts/shared/behaviors.js
index f2d3267ce..a78b53c84 100644
--- a/app/assets/javascripts/shared/behaviors.js
+++ b/app/assets/javascripts/shared/behaviors.js
@@ -88,6 +88,14 @@
         searchParams.set('tab', currentTab);
         history.pushState(null, null, `?${searchParams.toString()}`);
       });
+
+    // Allows users to navigate using the native browser back/forward buttons
+    // even when we manipulate the browser history with pushState()
+    $(window)
+      .unbind()
+      .on('popstate', function () {
+        location.reload();
+      });
   }
 
   document.addEventListener('turbolinks:load', function () {

From 0c6ae00d962d18680c78e3d31b830b1e6d120794 Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Mon, 17 Jun 2024 14:50:58 +0200
Subject: [PATCH 04/18] update changelog

---
 CHANGELOG | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 4968516d7..3a90ae93a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,8 +2,7 @@
   - Liquid: Make project-level collections available for Liquid syntax
   - Upgraded gems: nokogiri, rails, rexml
   - Bugs fixes:
-    - [entity]:
-      - [future tense verb] [bug fix]
+    - Navigation: Restore functionality of native browser back/forward buttons
     - Bug tracker items:
       - [item]
   - New integrations:

From f352a1c4ad3f944ba46db6c5c07ad749bae2e8d2 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Tue, 18 Jun 2024 17:47:04 +0800
Subject: [PATCH 05/18] Add AttachmentsCopier concern

---
 CHANGELOG                                     |  1 +
 .../concerns/attachments_copier.rb            | 19 +++++++++++++++++++
 app/controllers/evidence_controller.rb        |  3 +++
 app/controllers/notes_controller.rb           |  7 ++++++-
 app/models/attachment.rb                      |  2 ++
 spec/features/evidence_moving_spec.rb         | 10 +++++++++-
 spec/features/note_moving_spec.rb             | 10 +++++++++-
 7 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 app/controllers/concerns/attachments_copier.rb

diff --git a/CHANGELOG b/CHANGELOG
index 4968516d7..55c5c3f83 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,5 @@
 [v#.#.#] ([month] [YYYY])
+  - Attachments: Copy attachments when moving an evidence/note
   - Liquid: Make project-level collections available for Liquid syntax
   - Upgraded gems: nokogiri, rails, rexml
   - Bugs fixes:
diff --git a/app/controllers/concerns/attachments_copier.rb b/app/controllers/concerns/attachments_copier.rb
new file mode 100644
index 000000000..ff093a3f8
--- /dev/null
+++ b/app/controllers/concerns/attachments_copier.rb
@@ -0,0 +1,19 @@
+module AttachmentsCopier
+  def copy_attachments(record)
+    record.content.scan(Attachment::SCREENSHOT_REGEX).each do |screenshot_url|
+      _, _, _, _, project_id, node_id, filename, _ = screenshot_url
+
+      attachment = Attachment.find_by(filename: filename, node_id: record.node_id_was)
+
+      if attachment
+        new_attachment = attachment.copy_to(record.node)
+        new_url = screenshot_url[0].gsub(
+          /nodes\/[0-9]+\/attachments/,
+          "nodes/#{new_attachment.node_id}/attachments"
+        )
+
+        record.content = record.content.gsub(screenshot_url[0], new_url)
+      end
+    end
+  end
+end
diff --git a/app/controllers/evidence_controller.rb b/app/controllers/evidence_controller.rb
index e66247f42..1d78da371 100644
--- a/app/controllers/evidence_controller.rb
+++ b/app/controllers/evidence_controller.rb
@@ -1,4 +1,5 @@
 class EvidenceController < NestedNodeResourceController
+  include AttachmentsCopier
   include ConflictResolver
   include EvidenceHelper
   include LiquidEnabledResource
@@ -55,6 +56,8 @@ def update
       @evidence.assign_attributes(evidence_params)
       autogenerate_issue if evidence_params[:issue_id].blank?
 
+      copy_attachments(@evidence) if @evidence.node_changed?
+
       if @evidence.save
         track_updated(@evidence)
         check_for_edit_conflicts(@evidence, updated_at_before_save)
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index 895e094f0..71715460d 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -1,6 +1,7 @@
 # This controller exposes the REST operations required to manage the Note
 # resource.
 class NotesController < NestedNodeResourceController
+  include AttachmentsCopier
   include ConflictResolver
   include LiquidEnabledResource
   include Mentioned
@@ -44,7 +45,11 @@ def edit
   # Update the attributes of a Note
   def update
     updated_at_before_save = @note.updated_at.to_i
-    if @note.update(note_params)
+
+    @note.assign_attributes(note_params)
+    copy_attachments(@note) if @note.node_changed?
+
+    if @note.save
       track_updated(@note)
       check_for_edit_conflicts(@note, updated_at_before_save)
       # if the note has just been moved to another node, we must reload
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index f80c888bd..25e78e4dd 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -66,6 +66,8 @@ class Attachment < File
   AttachmentPwd = Rails.env.test? ? Rails.root.join('tmp', 'attachments') : Rails.root.join('attachments')
   FileUtils.mkdir_p(AttachmentPwd) unless File.exists?(AttachmentPwd)
 
+  SCREENSHOT_REGEX = /\!((https?:\/\/.+)|((\/pro)?\/projects\/(\d+)\/nodes\/(\d+)\/attachments\/(.+?)(\(.*\))?))\!/i
+
   # -- Class Methods  ---------------------------------------------------------
 
   def self.all(*args)
diff --git a/spec/features/evidence_moving_spec.rb b/spec/features/evidence_moving_spec.rb
index d0a211996..76a92f726 100644
--- a/spec/features/evidence_moving_spec.rb
+++ b/spec/features/evidence_moving_spec.rb
@@ -29,9 +29,13 @@ def create_node(label, parent = nil)
     click_move_evidence
   end
 
-  let(:current_evidence) { @evidence = create(:evidence, node: @node_5) }
+  let(:content) { "#[Description]#\nTest Evidence\n" }
+  let(:current_evidence) { @evidence = create(:evidence, content: content, node: @node_5) }
 
   describe 'moving an evidence to a different node' do
+    let(:attachment) { create(:attachment, node: @node_5) }
+    let(:content) { "#[Description]#\n!/projects/#{current_project.id}/nodes/#{@node_5.id}/attachments/#{attachment.filename}!\n" }
+
     before do
       within('#modal_move_evidence') do
         click_link @node_1.label
@@ -46,6 +50,10 @@ def create_node(label, parent = nil)
     it 'should redirect to evidence show path' do
       expect(current_path).to eq(project_node_evidence_path(current_project, @node_1, current_evidence))
     end
+
+    it 'should update the attachment reference to the new node' do
+      expect(current_evidence.reload.content).to include("nodes/#{@node_1.id}")
+    end
   end
 
   describe 'moving a evidence to a similar node', js: true do
diff --git a/spec/features/note_moving_spec.rb b/spec/features/note_moving_spec.rb
index 8b4890c67..53b5c08c6 100644
--- a/spec/features/note_moving_spec.rb
+++ b/spec/features/note_moving_spec.rb
@@ -30,9 +30,13 @@ def create_node(label, parent = nil)
     click_move_note
   end
 
-  let(:current_note) { @note = create(:note, node: @node_5) }
+  let(:text) { "#[Description]#\nTest Note\n" }
+  let(:current_note) { @note = create(:note, text: text, node: @node_5) }
 
   describe 'moving a note to a different node' do
+    let(:attachment) { create(:attachment, node: @node_5) }
+    let(:text) { "#[Description]#\n!/projects/#{current_project.id}/nodes/#{@node_5.id}/attachments/#{attachment.filename}!\n" }
+
     before do
       within('#modal_move_note') do
         click_link @node_1.label
@@ -47,6 +51,10 @@ def create_node(label, parent = nil)
     it 'should redirect to note show path' do
       expect(current_path).to eq(project_node_note_path(current_project, @node_1, current_note))
     end
+
+    it 'should update the attachment reference to the new node' do
+      expect(current_note.reload.content).to include("nodes/#{@node_1.id}")
+    end
   end
 
   describe 'moving a note to a similar node' do

From 2b28481b6675a63bf75db35ad961a93dbe675239 Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Tue, 18 Jun 2024 11:53:53 +0200
Subject: [PATCH 06/18] limit scope of eventLister to prevent N bindings

---
 app/assets/javascripts/shared/behaviors.js | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/shared/behaviors.js b/app/assets/javascripts/shared/behaviors.js
index a78b53c84..059d14f4e 100644
--- a/app/assets/javascripts/shared/behaviors.js
+++ b/app/assets/javascripts/shared/behaviors.js
@@ -81,8 +81,8 @@
     }
 
     // Update address bar with current tab param
-    $('[data-bs-toggle~=tab]')
-      .unbind()
+    $(parentElement)
+      .find('[data-bs-toggle~=tab]')
       .on('shown.bs.tab', function (e) {
         let currentTab = $(e.target).attr('href').substring(1);
         searchParams.set('tab', currentTab);
@@ -91,11 +91,9 @@
 
     // Allows users to navigate using the native browser back/forward buttons
     // even when we manipulate the browser history with pushState()
-    $(window)
-      .unbind()
-      .on('popstate', function () {
-        location.reload();
-      });
+    $(window).on('popstate', function () {
+      location.reload();
+    });
   }
 
   document.addEventListener('turbolinks:load', function () {

From 8645017b96fa5c06abebfd71495ed1dc706f4d27 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Tue, 18 Jun 2024 17:55:30 +0800
Subject: [PATCH 07/18] Appease rubocop

---
 app/models/attachment.rb | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 25e78e4dd..455afe832 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -14,7 +14,6 @@
 **
 =end
 
-
 # ==Description
 # This class in an abstraction layer to the <tt>attachments/</tt> folder. It allows
 # access to the folder content in a way that mimics the working of ActiveRecord
@@ -83,7 +82,7 @@ def self.count
   end
 
   def self.find_by(filename:, node_id:)
-    find(filename, conditions: { node_id: node_id } )
+    find(filename, conditions: { node_id: node_id })
   rescue StandardError
   end
 
@@ -93,17 +92,17 @@ def self.find(*args)
     dir = Dir.new(pwd)
 
     # makes the find request and stores it to resources
-    return_value = case args.first
+    case args.first
     when :all, :first, :last
       attachments = []
       if options[:conditions] && options[:conditions][:node_id]
         node_id = options[:conditions][:node_id].to_s
         raise "Node with ID=#{node_id} does not exist" unless Node.exists?(node_id)
-        if (File.exist?( File.join(pwd, node_id)))
+        if (File.exist?(File.join(pwd, node_id)))
           node_dir = Dir.new(pwd.join(node_id)).sort
           node_dir.each do |attachment|
             next unless (attachment =~ /^(.+)$/) == 0 && !File.directory?(pwd.join(node_id, attachment))
-            attachments << Attachment.new(:filename => $1, :node_id => node_id.to_i)
+            attachments << Attachment.new(filename: $1, node_id: node_id.to_i)
           end
         end
       else
@@ -112,7 +111,7 @@ def self.find(*args)
           node_dir = Dir.new(pwd.join(node)).sort
           node_dir.each do |attachment|
             next unless (attachment =~ /^(.+)$/) == 0 && !File.directory?(pwd.join(node, attachment))
-            attachments << Attachment.new(:filename => $1, :node_id => node.to_i)
+            attachments << Attachment.new(filename: $1, node_id: node.to_i)
           end
         end
         attachments.sort_by!(&:filename)
@@ -131,18 +130,17 @@ def self.find(*args)
       # in this routine we find the attachment by file name and node id
       filename = args.first
       attachments = []
-      raise "You need to supply a node id in the condition parameter" unless options[:conditions] && options[:conditions][:node_id]
+      raise 'You need to supply a node id in the condition parameter' unless options[:conditions] && options[:conditions][:node_id]
       node_id = options[:conditions][:node_id].to_s
       raise "Node with ID=#{node_id} does not exist" unless Node.exists?(node_id)
       node_dir = Dir.new(pwd.join(node_id)).sort
       node_dir.each do |attachment|
         next unless ((attachment =~ /^(.+)$/) == 0 && $1 == filename)
-        attachments << Attachment.new(:filename => $1, :node_id => node_id.to_i)
+        attachments << Attachment.new(filename: $1, node_id: node_id.to_i)
       end
       raise "Could not find Attachment with filename #{filename}" if attachments.empty?
       attachments.first
     end
-    return return_value
   end
 
   def self.model_name

From 8e8dc68ce5180e5d64c5cbc363e8cd8b63508114 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Wed, 19 Jun 2024 18:40:58 +0800
Subject: [PATCH 08/18] Add support for merge and add specs

---
 .../concerns/liquid_enabled_resource.rb       |  2 +-
 app/services/liquid_assigns_service.rb        | 16 -----
 app/services/liquid_cached_assigns.rb         | 32 +++++-----
 spec/services/liquid_assigns_service_spec.rb  | 59 ------------------
 spec/services/liquid_cached_assigns_spec.rb   | 61 +++++++++++++++++++
 5 files changed, 77 insertions(+), 93 deletions(-)
 delete mode 100644 app/services/liquid_assigns_service.rb
 delete mode 100644 spec/services/liquid_assigns_service_spec.rb
 create mode 100644 spec/services/liquid_cached_assigns_spec.rb

diff --git a/app/controllers/concerns/liquid_enabled_resource.rb b/app/controllers/concerns/liquid_enabled_resource.rb
index 04e94046f..aa0661e31 100644
--- a/app/controllers/concerns/liquid_enabled_resource.rb
+++ b/app/controllers/concerns/liquid_enabled_resource.rb
@@ -35,6 +35,6 @@ def project_assigns
     project = Project.find(params[:project_id])
     authorize! :use, project
 
-    LiquidAssignsService.new(project: project).assigns
+    LiquidCachedAssigns.new(project: project)
   end
 end
diff --git a/app/services/liquid_assigns_service.rb b/app/services/liquid_assigns_service.rb
deleted file mode 100644
index 0260060f3..000000000
--- a/app/services/liquid_assigns_service.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-class LiquidAssignsService
-  attr_accessor :project
-
-  def initialize(project:)
-    @project = project
-  end
-
-  def assigns
-    LiquidCachedAssigns.new(project: project)
-  end
-
-  private
-
-  def assigns_pro
-  end
-end
diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
index cba8cbe8e..25a49b564 100644
--- a/app/services/liquid_cached_assigns.rb
+++ b/app/services/liquid_cached_assigns.rb
@@ -4,35 +4,31 @@ class LiquidCachedAssigns < Hash
   attr_accessor :assigns, :project
 
   def initialize(project:)
-    @assigns = { 'project' => ProjectDrop.new(@project) }
+    @assigns = { 'project' => ProjectDrop.new(project) }
+    @assigns.merge!(assigns_pro)
+
     @project = project
   end
 
   def [](record_type)
-    return unless AVAILABLE_PROJECT_ASSIGNS.include?(record_type)
-
-    if assigns[record_type]
-      assigns[record_type]
-    else
-      assigns[record_type] = cached_drops(record_type)
-    end
+    assigns[record_type] ||= cached_drops(record_type)
   end
 
-  def inspect
-    assigns.inspect
+  def key?(key)
+    AVAILABLE_PROJECT_ASSIGNS.include?(key.to_s) || assigns.key?(key)
   end
 
-  def key?(msg)
-    AVAILABLE_PROJECT_ASSIGNS.include?(msg.to_s)
-  end
-
-  def respond_to?(msg)
-    return true if key?(msg)
-    super(msg)
+  def merge!(hash)
+    @assigns.merge!(hash)
+    self
   end
 
   private
 
+  def assigns_pro
+    {}
+  end
+
   def cached_drops(record_type)
     records = project_records(record_type)
 
@@ -47,6 +43,8 @@ def cached_drops(record_type)
   end
 
   def project_records(record_type)
+    return [] unless AVAILABLE_PROJECT_ASSIGNS.include?(record_type)
+
     case record_type
     when 'evidences'
       project.evidence
diff --git a/spec/services/liquid_assigns_service_spec.rb b/spec/services/liquid_assigns_service_spec.rb
deleted file mode 100644
index 5e1fd0796..000000000
--- a/spec/services/liquid_assigns_service_spec.rb
+++ /dev/null
@@ -1,59 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe LiquidAssignsService do
-  let!(:project) { create(:project) }
-
-  before do
-    node = create(:node, project: project)
-    issue = create(:issue, node: project.issue_library)
-    create(:evidence, issue: issue, node: node)
-    create(:note, node: node)
-    create(:tag)
-  end
-
-  describe '#project_assigns' do
-    context 'with the :text argument' do
-      LiquidAssignsService::AVAILABLE_PROJECT_ASSIGNS.each do |assign|
-        it "adds #{assign} to the project_assigns if present in the text" do
-          text = "#[Description]#\n {% for #{assign.singularize} in #{assign} %}{% endfor %}\n"
-          liquid_assigns = described_class.new(project: project, text: text).assigns
-
-          expect(liquid_assigns.keys).to include(assign)
-        end
-      end
-    end
-
-    context 'without the :text argument' do
-      let(:liquid_assigns) { described_class.new(project: project).assigns }
-
-      it 'builds a hash of liquid assigns' do
-        expect(liquid_assigns['project'].name).to eq(project.name)
-        expect(liquid_assigns['issues'].map(&:title)).to eq(project.issues.map(&:title))
-        expect(liquid_assigns['evidences'].map(&:title)).to eq(project.evidence.map(&:title))
-        expect(liquid_assigns['nodes'].map(&:label)).to eq(project.nodes.user_nodes.map(&:label))
-        expect(liquid_assigns['notes'].map(&:title)).to eq(project.notes.map(&:title))
-        expect(liquid_assigns['tags'].map(&:display_name)).to eq(project.tags.map(&:display_name))
-      end
-    end
-  end
-
-  context 'with pro records', skip: !defined?(Dradis::Pro)  do
-    let(:liquid_assigns) { described_class.new(project: project).assigns }
-
-    let!(:project) { create(:project, :with_team) }
-
-    before do
-      report_content = project.content_library
-      report_content.properties = { 'dradis.project' => project.name }
-      report_content.save
-
-      create(:content_block, project: project)
-    end
-
-    it 'builds a hash with Dradis::Pro assigns' do
-      expect(liquid_assigns['document_properties'].available_properties).to eq({ 'dradis.project' => project.name })
-      expect(liquid_assigns['team'].name).to eq(project.team.name)
-      expect(liquid_assigns['content_blocks'].map(&:content)).to eq(project.content_blocks.map(&:content))
-    end
-  end
-end
diff --git a/spec/services/liquid_cached_assigns_spec.rb b/spec/services/liquid_cached_assigns_spec.rb
new file mode 100644
index 000000000..b53ec6c29
--- /dev/null
+++ b/spec/services/liquid_cached_assigns_spec.rb
@@ -0,0 +1,61 @@
+require 'rails_helper'
+
+RSpec.describe LiquidCachedAssigns do
+  let!(:project) { create(:project) }
+  let(:liquid_assigns) { described_class.new(project: project) }
+
+  before do
+    node = create(:node, project: project)
+    issue = create(:issue, node: project.issue_library)
+    create(:evidence, issue: issue, node: node)
+    create(:note, node: node)
+    create(:tag)
+  end
+
+  context 'fetching an assign from an available collection' do
+    it 'lazily loads the assigns' do
+      expect(liquid_assigns.assigns.keys).to_not include(
+        %w{issues evidences nodes notes tags}
+      )
+    end
+
+    it 'builds a hash of liquid assigns' do
+      expect(liquid_assigns['project'].name).to eq(project.name)
+      expect(liquid_assigns['issues'].map(&:title)).to eq(project.issues.map(&:title))
+      expect(liquid_assigns['evidences'].map(&:title)).to eq(project.evidence.map(&:title))
+      expect(liquid_assigns['nodes'].map(&:label)).to eq(project.nodes.user_nodes.map(&:label))
+      expect(liquid_assigns['notes'].map(&:title)).to eq(project.notes.map(&:title))
+      expect(liquid_assigns['tags'].map(&:display_name)).to eq(project.tags.map(&:display_name))
+    end
+  end
+
+  context 'fetching an assign from a unavailable collection' do
+    it 'returns an empty array' do
+      expect(liquid_assigns['fake']).to be_empty
+    end
+  end
+
+  context 'with pro records', skip: !defined?(Dradis::Pro)  do
+    let!(:project) { create(:project, :with_team) }
+
+    before do
+      report_content = project.content_library
+      report_content.properties = { 'dradis.project' => project.name }
+      report_content.save
+
+      create(:content_block, project: project)
+    end
+
+    context 'fetching an assign from an available collection' do
+      it 'lazily loads the assigns' do
+        expect(liquid_assigns.assigns.keys).to_not include('content_blocks')
+      end
+
+      it 'builds a hash with Dradis::Pro assigns' do
+        expect(liquid_assigns['document_properties'].available_properties).to eq({ 'dradis.project' => project.name })
+        expect(liquid_assigns['team'].name).to eq(project.team.name)
+        expect(liquid_assigns['content_blocks'].map(&:content)).to eq(project.content_blocks.map(&:content))
+      end
+    end
+  end
+end

From 1a56cde52d4ccc386683345d71cc99e17527ff4f Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Wed, 19 Jun 2024 18:51:32 +0800
Subject: [PATCH 09/18] Reorder initialization

---
 app/services/liquid_cached_assigns.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
index 25a49b564..b1bd61079 100644
--- a/app/services/liquid_cached_assigns.rb
+++ b/app/services/liquid_cached_assigns.rb
@@ -4,10 +4,10 @@ class LiquidCachedAssigns < Hash
   attr_accessor :assigns, :project
 
   def initialize(project:)
+    @project = project
+
     @assigns = { 'project' => ProjectDrop.new(project) }
     @assigns.merge!(assigns_pro)
-
-    @project = project
   end
 
   def [](record_type)

From 2980a098270a0415388ef0fa77725a1a4f54f6b3 Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Wed, 19 Jun 2024 15:09:57 +0200
Subject: [PATCH 10/18] navigate with `Turbolinks` vs `location.reload()`

---
 app/assets/javascripts/shared/behaviors.js | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/shared/behaviors.js b/app/assets/javascripts/shared/behaviors.js
index 059d14f4e..6563983fd 100644
--- a/app/assets/javascripts/shared/behaviors.js
+++ b/app/assets/javascripts/shared/behaviors.js
@@ -86,14 +86,19 @@
       .on('shown.bs.tab', function (e) {
         let currentTab = $(e.target).attr('href').substring(1);
         searchParams.set('tab', currentTab);
-        history.pushState(null, null, `?${searchParams.toString()}`);
+        let urlWithTab = `?${searchParams.toString()}`;
+        history.replaceState(null, null, urlWithTab);
       });
 
     // Allows users to navigate using the native browser back/forward buttons
     // even when we manipulate the browser history with pushState()
-    $(window).on('popstate', function () {
-      location.reload();
-    });
+    $(window)
+      .off()
+      .on('popstate', function () {
+        if (location.search.length) {
+          Turbolinks.visit(location, { action: 'replace' });
+        }
+      });
   }
 
   document.addEventListener('turbolinks:load', function () {

From 28fee462776e26a3b962cce57f8f5c2082e2e34d Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Wed, 19 Jun 2024 15:14:30 +0200
Subject: [PATCH 11/18] add tab to history rather than replacing current
 history

---
 app/assets/javascripts/shared/behaviors.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/shared/behaviors.js b/app/assets/javascripts/shared/behaviors.js
index 6563983fd..29bdaa231 100644
--- a/app/assets/javascripts/shared/behaviors.js
+++ b/app/assets/javascripts/shared/behaviors.js
@@ -87,7 +87,7 @@
         let currentTab = $(e.target).attr('href').substring(1);
         searchParams.set('tab', currentTab);
         let urlWithTab = `?${searchParams.toString()}`;
-        history.replaceState(null, null, urlWithTab);
+        history.pushState(null, null, urlWithTab);
       });
 
     // Allows users to navigate using the native browser back/forward buttons

From cdaa6620b2d658a9b02df7cce79ce9674b6ffe92 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Fri, 21 Jun 2024 16:34:08 +0800
Subject: [PATCH 12/18] Add comment and implement #merge method

---
 app/services/liquid_cached_assigns.rb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
index b1bd61079..2d64744d7 100644
--- a/app/services/liquid_cached_assigns.rb
+++ b/app/services/liquid_cached_assigns.rb
@@ -14,10 +14,18 @@ def [](record_type)
     assigns[record_type] ||= cached_drops(record_type)
   end
 
+  # SEE: https://github.com/Shopify/liquid/blob/77bc56/lib/liquid/context.rb#L211
+  # Liquid is checking if the variable is present in the assigns hash by
+  # calling the `key?` method. Since we're lazily loading the keys, the variable
+  # may not yet be present in the assigns hash.
   def key?(key)
     AVAILABLE_PROJECT_ASSIGNS.include?(key.to_s) || assigns.key?(key)
   end
 
+  def merge(hash)
+    LiquidCachedAssigns.new(project: project).merge!(@assigns.merge(hash))
+  end
+
   def merge!(hash)
     @assigns.merge!(hash)
     self

From 1ba771f2840bb2df8ee23fdd33e9b6daf093c2aa Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Mon, 24 Jun 2024 14:28:27 +0800
Subject: [PATCH 13/18] Include only node notes in liquid notes

---
 app/services/liquid_cached_assigns.rb       | 3 +++
 spec/services/liquid_cached_assigns_spec.rb | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
index 2d64744d7..bf0c03b23 100644
--- a/app/services/liquid_cached_assigns.rb
+++ b/app/services/liquid_cached_assigns.rb
@@ -58,6 +58,9 @@ def project_records(record_type)
       project.evidence
     when 'nodes'
       project.nodes.user_nodes
+    when 'notes'
+      # FIXME - ISSUE/NOTE INHERITANCE
+      project.notes.includes(:node).where.not(node: { type_id: Node::Types::ISSUELIB })
     else
       project.send(record_type.to_sym)
     end
diff --git a/spec/services/liquid_cached_assigns_spec.rb b/spec/services/liquid_cached_assigns_spec.rb
index b53ec6c29..34889a466 100644
--- a/spec/services/liquid_cached_assigns_spec.rb
+++ b/spec/services/liquid_cached_assigns_spec.rb
@@ -20,11 +20,13 @@
     end
 
     it 'builds a hash of liquid assigns' do
+      issues = project.issues.map(&:title)
+
       expect(liquid_assigns['project'].name).to eq(project.name)
-      expect(liquid_assigns['issues'].map(&:title)).to eq(project.issues.map(&:title))
+      expect(liquid_assigns['issues'].map(&:title)).to eq(issues)
       expect(liquid_assigns['evidences'].map(&:title)).to eq(project.evidence.map(&:title))
       expect(liquid_assigns['nodes'].map(&:label)).to eq(project.nodes.user_nodes.map(&:label))
-      expect(liquid_assigns['notes'].map(&:title)).to eq(project.notes.map(&:title))
+      expect(liquid_assigns['notes'].map(&:title)).to eq(project.notes.map(&:title) - issues)
       expect(liquid_assigns['tags'].map(&:display_name)).to eq(project.tags.map(&:display_name))
     end
   end

From f44129cd44180fc05c9848ea34666275fdecefd6 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Mon, 24 Jun 2024 17:36:28 +0800
Subject: [PATCH 14/18] Handle attachments with spaces and duplicated
 attachments

---
 app/controllers/concerns/attachments_copier.rb | 15 ++++++++-------
 spec/features/evidence_moving_spec.rb          |  5 ++++-
 spec/features/note_moving_spec.rb              |  5 ++++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/controllers/concerns/attachments_copier.rb b/app/controllers/concerns/attachments_copier.rb
index ff093a3f8..7db9ec05e 100644
--- a/app/controllers/concerns/attachments_copier.rb
+++ b/app/controllers/concerns/attachments_copier.rb
@@ -1,18 +1,19 @@
 module AttachmentsCopier
   def copy_attachments(record)
-    record.content.scan(Attachment::SCREENSHOT_REGEX).each do |screenshot_url|
-      _, _, _, _, project_id, node_id, filename, _ = screenshot_url
+    record.content.scan(Attachment::SCREENSHOT_REGEX).each do |screenshot_path|
+      full_screenshot_path, _, _, _, project_id, node_id, filename, _ = screenshot_path
 
-      attachment = Attachment.find_by(filename: filename, node_id: record.node_id_was)
+      attachment = Attachment.find_by(filename: CGI::unescape(filename), node_id: record.node_id_was)
 
       if attachment
         new_attachment = attachment.copy_to(record.node)
-        new_url = screenshot_url[0].gsub(
-          /nodes\/[0-9]+\/attachments/,
-          "nodes/#{new_attachment.node_id}/attachments"
+        new_filename = ERB::Util.url_encode(new_attachment.filename)
+        new_path = full_screenshot_path.gsub(
+          /nodes\/[0-9]+\/attachments\/.+/,
+          "nodes/#{new_attachment.node_id}/attachments/#{new_filename}"
         )
 
-        record.content = record.content.gsub(screenshot_url[0], new_url)
+        record.content = record.content.gsub(full_screenshot_path, new_path)
       end
     end
   end
diff --git a/spec/features/evidence_moving_spec.rb b/spec/features/evidence_moving_spec.rb
index 76a92f726..ea01c8f0b 100644
--- a/spec/features/evidence_moving_spec.rb
+++ b/spec/features/evidence_moving_spec.rb
@@ -33,10 +33,13 @@ def create_node(label, parent = nil)
   let(:current_evidence) { @evidence = create(:evidence, content: content, node: @node_5) }
 
   describe 'moving an evidence to a different node' do
-    let(:attachment) { create(:attachment, node: @node_5) }
+    let(:attachment) { create(:attachment, filename: 'name with spaces.png', node: @node_5) }
     let(:content) { "#[Description]#\n!/projects/#{current_project.id}/nodes/#{@node_5.id}/attachments/#{attachment.filename}!\n" }
 
     before do
+      # Ensure this works with duplicated attachment
+      create(:attachment, filename: 'name with spaces.png', node: @node_1)
+
       within('#modal_move_evidence') do
         click_link @node_1.label
         click_submit
diff --git a/spec/features/note_moving_spec.rb b/spec/features/note_moving_spec.rb
index 53b5c08c6..836402a62 100644
--- a/spec/features/note_moving_spec.rb
+++ b/spec/features/note_moving_spec.rb
@@ -34,10 +34,13 @@ def create_node(label, parent = nil)
   let(:current_note) { @note = create(:note, text: text, node: @node_5) }
 
   describe 'moving a note to a different node' do
-    let(:attachment) { create(:attachment, node: @node_5) }
+    let(:attachment) { create(:attachment, filename: 'name with spaces.png', node: @node_5) }
     let(:text) { "#[Description]#\n!/projects/#{current_project.id}/nodes/#{@node_5.id}/attachments/#{attachment.filename}!\n" }
 
     before do
+      # Ensure this works with duplicated attachment
+      create(:attachment, filename: 'name with spaces.png', node: @node_1)
+
       within('#modal_move_note') do
         click_link @node_1.label
         click_submit

From 489b982d4387894490c86ef25d9d13c06d5151d7 Mon Sep 17 00:00:00 2001
From: Matt Budz <matt.budz@protonmail.com>
Date: Mon, 24 Jun 2024 18:11:12 +0200
Subject: [PATCH 15/18] set data in pushState to load page with tab param

---
 app/assets/javascripts/shared/behaviors.js | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/shared/behaviors.js b/app/assets/javascripts/shared/behaviors.js
index 29bdaa231..9aa1e52a0 100644
--- a/app/assets/javascripts/shared/behaviors.js
+++ b/app/assets/javascripts/shared/behaviors.js
@@ -87,17 +87,11 @@
         let currentTab = $(e.target).attr('href').substring(1);
         searchParams.set('tab', currentTab);
         let urlWithTab = `?${searchParams.toString()}`;
-        history.pushState(null, null, urlWithTab);
-      });
-
-    // Allows users to navigate using the native browser back/forward buttons
-    // even when we manipulate the browser history with pushState()
-    $(window)
-      .off()
-      .on('popstate', function () {
-        if (location.search.length) {
-          Turbolinks.visit(location, { action: 'replace' });
-        }
+        history.pushState(
+          { turbolinks: true, url: urlWithTab },
+          '',
+          urlWithTab
+        );
       });
   }
 

From 33256b590c1bfb79dc103401cb9deb4ccabcc318 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Tue, 25 Jun 2024 16:26:05 +0800
Subject: [PATCH 16/18] Use url_encoded_filename method

---
 app/controllers/concerns/attachments_copier.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/concerns/attachments_copier.rb b/app/controllers/concerns/attachments_copier.rb
index 7db9ec05e..3ad9f5db7 100644
--- a/app/controllers/concerns/attachments_copier.rb
+++ b/app/controllers/concerns/attachments_copier.rb
@@ -7,7 +7,7 @@ def copy_attachments(record)
 
       if attachment
         new_attachment = attachment.copy_to(record.node)
-        new_filename = ERB::Util.url_encode(new_attachment.filename)
+        new_filename = new_attachment.url_encoded_filename
         new_path = full_screenshot_path.gsub(
           /nodes\/[0-9]+\/attachments\/.+/,
           "nodes/#{new_attachment.node_id}/attachments/#{new_filename}"

From f7ec2af0c016a06ff37b523901cd7644a3b04d9b Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Wed, 26 Jun 2024 20:25:21 +0800
Subject: [PATCH 17/18] Simplify project.notes query

---
 app/services/liquid_cached_assigns.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
index bf0c03b23..9e74dc0f1 100644
--- a/app/services/liquid_cached_assigns.rb
+++ b/app/services/liquid_cached_assigns.rb
@@ -60,7 +60,7 @@ def project_records(record_type)
       project.nodes.user_nodes
     when 'notes'
       # FIXME - ISSUE/NOTE INHERITANCE
-      project.notes.includes(:node).where.not(node: { type_id: Node::Types::ISSUELIB })
+      project.notes.where.not(node_id: project.issue_library.id)
     else
       project.send(record_type.to_sym)
     end

From 604b95b84c129fa5717e46b7697c8b9f88b054c2 Mon Sep 17 00:00:00 2001
From: Aaron Manaloto <mail@aapomm.io>
Date: Wed, 26 Jun 2024 21:55:07 +0800
Subject: [PATCH 18/18] Update merge method

---
 app/services/liquid_cached_assigns.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/services/liquid_cached_assigns.rb b/app/services/liquid_cached_assigns.rb
index 9e74dc0f1..5f1e82562 100644
--- a/app/services/liquid_cached_assigns.rb
+++ b/app/services/liquid_cached_assigns.rb
@@ -23,7 +23,9 @@ def key?(key)
   end
 
   def merge(hash)
-    LiquidCachedAssigns.new(project: project).merge!(@assigns.merge(hash))
+    lca = LiquidCachedAssigns.new(project: project)
+    lca.assigns = @assigns.merge(hash)
+    lca
   end
 
   def merge!(hash)