[Rt-commit] rt branch, 4.4/shared-setting-txn, created. rt-4.4.4-127-g92ead9ceb1

? sunnavy sunnavy at bestpractical.com
Mon Jul 13 20:05:59 EDT 2020


The branch, 4.4/shared-setting-txn has been created
        at  92ead9ceb1c8e5971cb5308706a9af1ddf6b6bb0 (commit)

- Log -----------------------------------------------------------------
commit e204e3ecf14187bd7929389d893e8a6ea92c571f
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed Jan 15 17:39:56 2020 +0800

    Sort hashes in attribute content to avoid unnecessary updates
    
    For hashes containing the same data, nfreezed version could be different
    because of arbitrary order of hash keys. This is to avoid setting the
    same content value unnessarily.

diff --git a/lib/RT/Attribute.pm b/lib/RT/Attribute.pm
index e8d1b47188..823a09928d 100644
--- a/lib/RT/Attribute.pm
+++ b/lib/RT/Attribute.pm
@@ -267,7 +267,8 @@ sub Content {
 sub _SerializeContent {
     my $self = shift;
     my $content = shift;
-        return( encode_base64(nfreeze($content))); 
+    local $Storable::canonical = 1;
+    return( encode_base64(nfreeze($content)));
 }
 
 

commit 1174d7857002922342b3502d2fe17ffb3c5a783a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed Jan 15 17:48:46 2020 +0800

    Add transaction records for dashboard/savedsearch changes

diff --git a/lib/RT/Attribute.pm b/lib/RT/Attribute.pm
index 823a09928d..c2471f3e5e 100644
--- a/lib/RT/Attribute.pm
+++ b/lib/RT/Attribute.pm
@@ -145,6 +145,7 @@ sub Create {
                 Content => '',
                 ContentType => '',
                 Object => undef,
+                RecordTransaction => undef,
                 @_);
 
     if ($args{Object} and UNIVERSAL::can($args{Object}, 'Id')) {
@@ -181,15 +182,43 @@ sub Create {
         $args{'ContentType'} = 'storable';
     }
 
-    $self->SUPER::Create(
-                         Name => $args{'Name'},
-                         Content => $args{'Content'},
-                         ContentType => $args{'ContentType'},
-                         Description => $args{'Description'},
-                         ObjectType => $args{'ObjectType'},
-                         ObjectId => $args{'ObjectId'},
-);
+    $args{'RecordTransaction'} //= 1 if $args{'Name'} =~ /^(?:SavedSearch|Dashboard|Subscription)$/;
 
+    $RT::Handle->BeginTransaction if $args{'RecordTransaction'};
+    my @return = $self->SUPER::Create(
+        Name        => $args{'Name'},
+        Content     => $args{'Content'},
+        ContentType => $args{'ContentType'},
+        Description => $args{'Description'},
+        ObjectType  => $args{'ObjectType'},
+        ObjectId    => $args{'ObjectId'},
+    );
+
+
+    if ( $args{'RecordTransaction'} ) {
+        if ( $return[0] ) {
+            my ( $ret, $msg ) = $self->_NewTransaction( Type => 'Create' );
+            if ($ret) {
+                ( $ret, $msg ) = $self->AddAttribute(
+                    Name    => 'ContentHistory',
+                    Content => $args{'ContentType'} eq 'storable'
+                    ? $self->_DeserializeContent( $args{'Content'} )
+                    : $args{'Content'},
+                );
+            }
+
+            @return = ( $ret, $msg ) unless $ret;
+        }
+
+        if ( $return[0] ) {
+            $RT::Handle->Commit;
+        }
+        else {
+            $RT::Handle->Rollback;
+        }
+    }
+
+    return wantarray ? @return : $return[0];
 }
 
 
@@ -374,11 +403,42 @@ sub Object {
 
 sub Delete {
     my $self = shift;
-    unless ($self->CurrentUserHasRight('delete')) {
-        return (0,$self->loc('Permission Denied'));
+    my %args = (
+        RecordTransaction => undef,
+        @_,
+    );
+
+    unless ( $self->CurrentUserHasRight('delete') ) {
+        return ( 0, $self->loc('Permission Denied') );
+    }
+
+    # Get values even if current user doesn't have right to see
+    $args{'RecordTransaction'} //= 1 if $self->__Value('Name') =~ /^(?:SavedSearch|Dashboard|Subscription)$/;
+
+    $RT::Handle->BeginTransaction if $args{'RecordTransaction'};
+
+    my @return = $self->SUPER::Delete(@_);
+
+    if ( $args{'RecordTransaction'} ) {
+        if ( $return[0] ) {
+            my $txn = RT::Transaction->new( $self->CurrentUser );
+            my ( $ret, $msg ) = $txn->Create(
+                ObjectId   => $self->Id,
+                ObjectType => ref($self),
+                Type       => 'Delete',
+            );
+            @return = ( $ret, $msg ) unless $ret;
+        }
+
+        if ( $return[0] ) {
+            $RT::Handle->Commit;
+        }
+        else {
+            $RT::Handle->Rollback;
+        }
     }
 
-    return($self->SUPER::Delete(@_));
+    return @return;
 }
 
 
@@ -396,12 +456,100 @@ sub _Value {
 
 sub _Set {
     my $self = shift;
-    unless ($self->CurrentUserHasRight('update')) {
+    my %args = (
+        Field             => undef,
+        Value             => undef,
+        RecordTransaction => undef,
+        TransactionType   => 'Set',
+        @_
+    );
 
-        return (0,$self->loc('Permission Denied'));
+    unless ( $self->CurrentUserHasRight('update') ) {
+        return ( 0, $self->loc('Permission Denied') );
+    }
+
+    # Get values even if current user doesn't have right to see
+    $args{'RecordTransaction'} //= 1 if $self->__Value('Name') =~ /^(?:SavedSearch|Dashboard|Subscription)$/;
+    my $old_value = $self->__Value( $args{'Field'} ) if $args{'RecordTransaction'};
+
+    $RT::Handle->BeginTransaction if $args{'RecordTransaction'};
+
+    # Set the new value
+    my @return = $self->SUPER::_Set(
+        Field => $args{'Field'},
+        Value => $args{'Value'},
+    );
+
+    if ( $args{'RecordTransaction'} ) {
+        if ( $return[0] ) {
+            my ( $new_ref, $old_ref );
+
+            my %opt = (
+                Type  => $args{'TransactionType'},
+                Field => $args{'Field'},
+            );
+            if ( $args{'Field'} eq 'Content' ) {
+
+                $opt{ReferenceType} = 'RT::Attribute';
+
+                my $attrs = $self->Attributes;
+                $attrs->Limit( FIELD => 'Name', VALUE => 'ContentHistory' );
+                $attrs->OrderByCols( { FIELD => 'id', ORDER => 'DESC' } );
+                if ( my $old_content = $attrs->First ) {
+                    $opt{OldReference} = $old_content->id;
+                }
+                else {
+                    RT->Logger->debug("Couldn't find ContentHistory, creating one from old value");
+                    my ( $ret, $msg ) = $self->AddAttribute(
+                        Name    => 'ContentHistory',
+                        Content => $self->__Value('ContentType') eq 'storable'
+                        ? $self->_DeserializeContent($old_value)
+                        : $old_value,
+                    );
+                    if ($ret) {
+                        $opt{OldReference} = $ret;
+                    }
+                    else {
+                        @return = ( $ret, $msg );
+                    }
+                }
+
+                if ( $return[0] ) {
+                    my ( $ret, $msg ) = $self->AddAttribute(
+                        Name    => 'ContentHistory',
+                        Content => $self->__Value('ContentType') eq 'storable'
+                        ? $self->_DeserializeContent( $args{'Value'} )
+                        : $args{'Value'},
+                    );
+
+                    if ($ret) {
+                        $opt{NewReference} = $ret;
+                    }
+                    else {
+                        @return = ( $ret, $msg );
+                    }
+                }
+            }
+            else {
+                $opt{'OldValue'} = $old_value;
+                $opt{'NewValue'} = $args{'Value'};
+            }
+
+            if ( $return[0] ) {
+                my ( $ret, $msg ) = $self->_NewTransaction(%opt);
+                @return = ( $ret, $msg ) unless $ret;
+            }
+        }
+
+        if ( $return[0] ) {
+            $RT::Handle->Commit;
+        }
+        else {
+            $RT::Handle->Rollback;
+        }
     }
-    return($self->SUPER::_Set(@_));
 
+    return wantarray ? @return : $return[0];
 }
 
 

commit bde1b96903253bbb914b60baa9e58a693b47adfe
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Fri Jan 17 04:19:48 2020 +0800

    Include related transactions for attribute serialization

diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 3503f67f30..3c2a83d9e7 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -2429,6 +2429,7 @@ sub FindDependencies {
         or $self->isa("RT::Article")
         or $self->isa("RT::Asset")
         or $self->isa("RT::Catalog")
+        or $self->isa("RT::Attribute")
         or $self->isa("RT::Queue") )
     {
         $objs = RT::Transactions->new( $self->CurrentUser );

commit 92ead9ceb1c8e5971cb5308706a9af1ddf6b6bb0
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Jul 14 03:49:40 2020 +0800

    Store the attribute Name in the ContentHistory's Description field
    
    With this, we can easily identify the type of shared settings like
    SavedSearch, Dashboard, etc.

diff --git a/lib/RT/Attribute.pm b/lib/RT/Attribute.pm
index c2471f3e5e..643e31c717 100644
--- a/lib/RT/Attribute.pm
+++ b/lib/RT/Attribute.pm
@@ -200,8 +200,9 @@ sub Create {
             my ( $ret, $msg ) = $self->_NewTransaction( Type => 'Create' );
             if ($ret) {
                 ( $ret, $msg ) = $self->AddAttribute(
-                    Name    => 'ContentHistory',
-                    Content => $args{'ContentType'} eq 'storable'
+                    Name        => 'ContentHistory',
+                    Description => $self->Name,
+                    Content     => $args{'ContentType'} eq 'storable'
                     ? $self->_DeserializeContent( $args{'Content'} )
                     : $args{'Content'},
                 );
@@ -501,8 +502,9 @@ sub _Set {
                 else {
                     RT->Logger->debug("Couldn't find ContentHistory, creating one from old value");
                     my ( $ret, $msg ) = $self->AddAttribute(
-                        Name    => 'ContentHistory',
-                        Content => $self->__Value('ContentType') eq 'storable'
+                        Name        => 'ContentHistory',
+                        Description => $self->Name,
+                        Content     => $self->__Value('ContentType') eq 'storable'
                         ? $self->_DeserializeContent($old_value)
                         : $old_value,
                     );
@@ -516,8 +518,9 @@ sub _Set {
 
                 if ( $return[0] ) {
                     my ( $ret, $msg ) = $self->AddAttribute(
-                        Name    => 'ContentHistory',
-                        Content => $self->__Value('ContentType') eq 'storable'
+                        Name        => 'ContentHistory',
+                        Description => $self->Name,
+                        Content     => $self->__Value('ContentType') eq 'storable'
                         ? $self->_DeserializeContent( $args{'Value'} )
                         : $args{'Value'},
                     );

-----------------------------------------------------------------------


More information about the rt-commit mailing list