Verified Commit 0c958868 authored by Bob Van Landuyt's avatar Bob Van Landuyt 💬 Committed by GitLab
Browse files

Merge branch 'fix/ux-sli-block-given' into 'master' 

fix: Do not return self from User Experience SLI methods when block given

See merge request !266

Merged-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Approved-by: Bob Van Landuyt's avatarBob Van Landuyt <bob@gitlab.com>
Reviewed-by: default avatarGitLab Duo <gitlab-duo@gitlab.com>
Co-authored-by: Hercules Merscher's avatarHercules Merscher <hmerscher@gitlab.com>
parents 07c30231 6cc38a97

lib/labkit/user_experience_sli/experience.rb

+5 −6
Original line number Diff line number Diff line
@@ -61,7 +61,8 @@ module Labkit 
      #

      # @yield [self] When a block is provided, the experience will be completed automatically.

      # @param extra [Hash] Additional data to include in the log event

      # @return [self]

      # @return [self] When no block is given.

      # @return [Object] The result of the block when a block is given.

      # @raise [UserExperienceError] If the block raises an error.

      #

      # Usage:
@@ -104,7 +105,9 @@ module Labkit 
      # Resume the User Experience.

      #

      # @yield [self] When a block is provided, the experience will be completed automatically.

      # @param extra [Hash] Additional data to include in the log

      # @param extra [Hash] Additional data to include in the log event

      # @return [self] When no block is given.

      # @return [Object] The result of the block when a block is given.

      def resume(**extra, &)

        return self unless ensure_started!
@@ -190,7 +193,6 @@ module Labkit 
      end



      def completable(**extra, &)

        begin

        yield self

      rescue StandardError => e

        error!(e)
@@ -199,9 +201,6 @@ module Labkit 
        complete(**extra)

      end



        self

      end



      def ensure_incomplete!

        return true if @end_time.nil?

lib/labkit/user_experience_sli/null.rb

+4 −2
Original line number Diff line number Diff line
@@ -11,12 +11,14 @@ module Labkit 
      def id = 'null'



      def start(**_extra)

        yield self if block_given?

        return yield(self) if block_given?



        self

      end



      def resume(**_extra)

        yield self if block_given?

        return yield(self) if block_given?



        self

      end

spec/labkit/user_experience_sli/experience_spec.rb

+4 −4
Original line number Diff line number Diff line
@@ -143,8 +143,8 @@ RSpec.describe Labkit::UserExperienceSli::Experience, :with_metrics_config do 
    it_behaves_like 'supports extra parameters', :start



    context 'when block is given' do

      it 'returns itself' do

        expect(experience.start { |_xp| 1 + 1 }).to be(experience)

      it 'returns the result of the block' do

        expect(experience.start { |_xp| 1 + 1 }).to eq(2)

      end



      it 'starts and automatically ends the experience' do
@@ -218,8 +218,8 @@ RSpec.describe Labkit::UserExperienceSli::Experience, :with_metrics_config do 
      it_behaves_like 'supports extra parameters', :resume



      context 'when block is given' do

        it 'returns itself' do

          expect(experience.resume { |_xp| 1 + 1 }).to be(experience)

        it 'returns the result of the block' do

          expect(experience.resume { |_xp| 1 + 1 }).to eq(2)

        end



        it 'resumes and automatically completes the experience' do

spec/labkit/user_experience_sli/null_spec.rb

+8 −0
Original line number Diff line number Diff line
@@ -52,6 +52,10 @@ RSpec.describe Labkit::UserExperienceSli::Null do 
      extra_attrs = { user_id: 456, session_id: 'abc123' }

      expect { |b| experience.start(**extra_attrs, &b) }.to yield_with_args(experience)

    end



    it 'returns the result of the block' do

      expect(experience.start { 42 }).to eq(42)

    end

  end



  describe '#resume' do
@@ -72,6 +76,10 @@ RSpec.describe Labkit::UserExperienceSli::Null do 
      extra_attrs = { user_id: 999, trace_id: 'trace-456' }

      expect { |b| experience.resume(**extra_attrs, &b) }.to yield_with_args(experience)

    end



    it 'returns the result of the block' do

      expect(experience.resume { 42 }).to eq(42)

    end

  end



  describe '#rehydrate' do