diff --git a/lib/NGCP/Panel/Role/API/Subscribers.pm b/lib/NGCP/Panel/Role/API/Subscribers.pm index 7b0ac4b5f2..97471c347e 100644 --- a/lib/NGCP/Panel/Role/API/Subscribers.pm +++ b/lib/NGCP/Panel/Role/API/Subscribers.pm @@ -391,11 +391,11 @@ sub prepare_resource { } my $absent_ids; ($groupmembers,$absent_ids) = NGCP::Panel::Utils::Subscriber::get_pbx_subscribers_ordered_by_ids( - c => $c, - schema => $schema, - ids => $resource->{pbx_groupmember_ids}, - customer_id => $resource->{customer_id}, - is_group => 0, + c => $c, + schema => $schema, + ids => $resource->{pbx_groupmember_ids}, + customer_id => $resource->{customer_id}, + is_group => 0, sync_with_ids => 1, ) ; if($absent_ids){ diff --git a/lib/NGCP/Panel/Utils/Subscriber.pm b/lib/NGCP/Panel/Utils/Subscriber.pm index c0cc3ba768..adad59dae2 100644 --- a/lib/NGCP/Panel/Utils/Subscriber.pm +++ b/lib/NGCP/Panel/Utils/Subscriber.pm @@ -3,6 +3,9 @@ use strict; use warnings; use Sipwise::Base; + +use NGCP::Panel::Utils::Generic qw(:all); + use DBIx::Class::Exception; use String::MkPasswd; use NGCP::Panel::Utils::DateTime; @@ -441,6 +444,7 @@ sub get_pbx_subscribers_ordered_by_ids{ } return wantarray ? (\@items, (( 0 < @absent_items_ids) ? \@absent_items_ids : undef )) : \@items; } + sub get_subscriber_pbx_items{ my %params = @_; @@ -495,71 +499,116 @@ sub manage_pbx_groups{ my $c = $params{c}; my $schema = $params{schema} // $c->model('DB'); my $group_ids = $params{group_ids} // []; + my $groups = $params{groups} // []; my $groupmember_ids = $params{groupmember_ids} // []; + my $groupmembers = $params{groupmembers} // []; my $subscriber = $params{subscriber}; my $customer = $params{customer} // $subscriber->contract; - my $groups = $params{groups} // ( @$group_ids ? get_pbx_subscribers_ordered_by_ids( - c => $c, - schema => $schema, - ids => $group_ids, - customer_id => $customer->id, - is_group => 1, - ) : [] ); - my $groupmembers = $params{groupmembers} // ( @$groupmember_ids ? get_pbx_subscribers_ordered_by_ids( - c => $c, - schema => $schema, - ids => $groupmember_ids, - customer_id => $customer->id, - is_group => 0, - ) : [] ); - - #delete all old groups, to support correct order my $prov_subscriber = $subscriber->provisioning_voip_subscriber; - my $subscriber_uri = get_pbx_group_member_name( subscriber => $subscriber ); - my $member_preferences_rs = NGCP::Panel::Utils::Preferences::get_usr_preference_rs( - c => $c, - attribute => 'cloud_pbx_hunt_group', - )->search_rs({ - subscriber_id => { -in => [ $prov_subscriber->voip_pbx_groups->get_column('group_id')->all ] }, - value => $subscriber_uri, - }); - $member_preferences_rs->delete; - $prov_subscriber->voip_pbx_groups->delete; + my $ids_existent = get_subscriber_pbx_items_ids( c => $c, subscriber => $subscriber ); - #create new groups - foreach my $group(@{ $groups }) { - my $group_prov_subscriber = $group->provisioning_voip_subscriber; - next unless( $group_prov_subscriber && $group_prov_subscriber->is_pbx_group ); - $prov_subscriber->voip_pbx_groups->create({ - group_id => $group_prov_subscriber->id, - }); - my $preferences_rs = NGCP::Panel::Utils::Preferences::get_usr_preference_rs( - c => $c, - attribute => 'cloud_pbx_hunt_group', - prov_subscriber => $group_prov_subscriber, - ); - $preferences_rs->create({ value => $subscriber_uri }); - } + if( !$prov_subscriber->is_pbx_group ){ + if(!@$group_ids && @$groups){ + $group_ids = [ map {$_->id} @$groups ]; + } + #Returns 0 if the structures differ, else returns 1. + if(!compare($ids_existent, $group_ids)){ + $c->log->debug('Old and new groups differ, apply changes'); - #delete old members to support correct order - $prov_subscriber->voip_pbx_group_members->delete; - my $group_preferences_rs = NGCP::Panel::Utils::Preferences::get_usr_preference_rs( - c => $c, - attribute => 'cloud_pbx_hunt_group', - prov_subscriber => $prov_subscriber, - ); - $group_preferences_rs->delete; - - foreach my $member(@{ $groupmembers }) { - my $member_prov_subscriber = $member->provisioning_voip_subscriber; - next unless( $member_prov_subscriber && !$member_prov_subscriber->is_pbx_group ); - my $member_uri = get_pbx_group_member_name( subscriber => $member ); - $prov_subscriber->voip_pbx_group_members->create({ - subscriber_id => $member_prov_subscriber->id, - }); - $group_preferences_rs->create({ value => $member_uri }); + $c->log->debug('Existent groups:'.join(',',@$ids_existent)); + $c->log->debug('Requested groups:'.join(',',@$group_ids)); + + my(@added_ids, @deleted_ids, %existent_hash, %requested_hash); + @requested_hash{@$group_ids} = @$group_ids; + @existent_hash{@$ids_existent} = @$ids_existent; + @added_ids = grep { !exists $existent_hash{$_} } keys %requested_hash; + @deleted_ids = grep { !exists $requested_hash{$_} } keys %existent_hash; + + my $subscriber_uri = get_pbx_group_member_name( subscriber => $subscriber ); + + + if(scalar @deleted_ids){ + #delete all old groups, to support correct order + $c->log->debug('Delete groups:'.join(',',@deleted_ids)); + my $member_preferences_rs = NGCP::Panel::Utils::Preferences::get_usr_preference_rs( + c => $c, + attribute => 'cloud_pbx_hunt_group', + )->search_rs({ + subscriber_id => { -in => [ @deleted_ids ] }, + value => $subscriber_uri, + }); + + $member_preferences_rs->delete; + $prov_subscriber->voip_pbx_groups->search_rs( { group_id => { -in => [@deleted_ids] } } )->delete; + } + if(scalar @added_ids){ + $c->log->debug('Added groups:'.join(',',@added_ids)); + my $groups_added = @$group_ids ? get_pbx_subscribers_ordered_by_ids( + c => $c, + schema => $schema, + ids => \@added_ids, + customer_id => $customer->id, + is_group => 1, + ) : [] ; + + #create new groups_added + foreach my $group(@{ $groups_added }) { + my $group_prov_subscriber = $group->provisioning_voip_subscriber; + next unless( $group_prov_subscriber && $group_prov_subscriber->is_pbx_group ); + $prov_subscriber->voip_pbx_groups->create({ + group_id => $group_prov_subscriber->id, + }); + my $preferences_rs = NGCP::Panel::Utils::Preferences::get_usr_preference_rs( + c => $c, + attribute => 'cloud_pbx_hunt_group', + prov_subscriber => $group_prov_subscriber, + ); + $preferences_rs->create({ value => $subscriber_uri }); + } + }{ + $c->log->debug('No groups were added.'); + } + }else{ + $c->log->debug('Old and new groups are the same'); + } + }else{ + if(!@$groupmember_ids && @$groupmembers){ + $groupmember_ids = [ map {$_->id} @$groupmembers ]; + } + #Returns 0 if the structures differ, else returns 1. + if(!compare($ids_existent, $groupmember_ids)){ + $c->log->debug('Old and new group members differ, apply changes'); + my $groupmembers = @$groupmember_ids ? get_pbx_subscribers_ordered_by_ids( + c => $c, + schema => $schema, + ids => $groupmember_ids, + customer_id => $customer->id, + is_group => 0, + ) : [] ; + + #delete old members to support correct order + $prov_subscriber->voip_pbx_group_members->delete; + my $group_preferences_rs = NGCP::Panel::Utils::Preferences::get_usr_preference_rs( + c => $c, + attribute => 'cloud_pbx_hunt_group', + prov_subscriber => $prov_subscriber, + ); + $group_preferences_rs->delete; + + foreach my $member(@{ $groupmembers }) { + my $member_prov_subscriber = $member->provisioning_voip_subscriber; + next unless( $member_prov_subscriber && !$member_prov_subscriber->is_pbx_group ); + my $member_uri = get_pbx_group_member_name( subscriber => $member ); + $prov_subscriber->voip_pbx_group_members->create({ + subscriber_id => $member_prov_subscriber->id, + }); + $group_preferences_rs->create({ value => $member_uri }); + } + }else{ + $c->log->debug('Old and new group members are the same'); + } } } sub get_pbx_group_member_name{ diff --git a/t/api-rest/api-subscribers.t b/t/api-rest/api-subscribers.t index 50c80cab50..38b6ad7c5a 100644 --- a/t/api-rest/api-subscribers.t +++ b/t/api-rest/api-subscribers.t @@ -62,7 +62,7 @@ $fake_data->set_data_from_script({ my $fake_data_processed = $fake_data->process('subscribers'); my $pilot = $test_machine->get_item_hal('subscribers','/api/subscribers/?customer_id='.$fake_data_processed->{customer_id}.'&'.'is_pbx_pilot=1'); -if($pilot->{content}->{total_count} > 0){ +if((exists $pilot->{total_count} && $pilot->{total_count}) || $pilot->{content}->{total_count} > 0){ $fake_data_processed->{is_pbx_pilot} = 0; #remove pilot aliases to don't intersect with them. On subscriber termination admin adopt numbers, see ticket#4967 $test_machine->request_patch( [ { op => 'replace', path => '/alias_numbers', value => [] } ], $pilot->{location} ); @@ -98,8 +98,8 @@ my $remote_config = $test_machine->init_catalyst_config; } { # create new subscribers from DATA_ITEM. Item is not created in the fake_data->process. - $test_machine->check_create_correct( 1, sub{ - $_[0]->{username} .= time().'_'.$_[1]->{i} ; + $test_machine->check_create_correct( 1, sub{ + $_[0]->{username} .= time().'_'.$_[1]->{i} ; } ); $test_machine->check_bundle(); $test_machine->check_get2put(undef,{}); @@ -180,10 +180,10 @@ my $remote_config = $test_machine->init_catalyst_config; #remove pilot aliases to don't intersect with them. On subscriber termination admin adopt numbers, see ticket#4967 $pilot and $test_machine->request_patch( [ { op => 'replace', path => '/alias_numbers', value => [] } ], $pilot->{location} ); } -{ +if($remote_config->{config}->{features}->{cloudpbx}){ + { #18601 - diag("18601: config->features->cloudpbx: ".$remote_config->{config}->{features}->{cloudpbx}.";\n"); - if($remote_config->{config}->{features}->{cloudpbx}){ + diag("18601: config->features->cloudpbx: ".$remote_config->{config}->{features}->{cloudpbx}.";\n"); my $groups = $test_machine->check_create_correct( 3, sub{ my $num = $_[1]->{i}; $_[0]->{username} .= time().'_18601_'.$num ; @@ -210,16 +210,35 @@ my $remote_config = $test_machine->init_catalyst_config; $members->[0]->{content}->{pbx_group_ids} = [map { $groups->[$_]->{content}->{id} } (2,1)]; diag("2. Check that member will return groups as they were specified"); - ($member_put,$member_get) = $test_machine->check_put2get($members->[0]); - + #($member_put,$member_get) = $test_machine->check_put2get($members->[0]); + my ($res,$content,$request) = $test_machine->request_put(@{$members->[0]}{qw/content location/}); + $test_machine->http_code_msg(200, "PUT of the members groups was successful", $res, $content); + $groups->[1]->{content}->{pbx_groupmember_ids} = [map { $members->[$_]->{content}->{id} } (2,1,0)]; diag("3. Check that group will return members as they were specified"); my($group_put,$group_get) = $test_machine->check_put2get($groups->[1]); - + $groups->[1]->{content}->{pbx_groupmember_ids} = []; diag("4. Check that group will return empty members after put members empty"); my($group_put,$group_get) = $test_machine->check_put2get($groups->[1]); +#5415 WF + diag("5415: check that groups management doesn't change members order;\n"); + diag("5415:Set members order for the group;\n"); + $groups->[0]->{content}->{pbx_groupmember_ids} = [ map { $members->[$_]->{content}->{id} } ( 0, 2, 1 ) ]; + $test_machine->check_put2get($groups->[0]); + + diag("5415:Touch one of the members;\n"); + $members->[0]->{content}->{pbx_group_ids} = [ map { $groups->[$_]->{content}->{id} } (2,1)]; + my($res,$content) = $test_machine->request_put(@{$members->[0]}{qw/content location/}); + $test_machine->http_code_msg(200, "PUT for groups was successful", $res, $content); + + diag("5415:Check members order in the group;\n"); + my(undef, $group_get_after) = $test_machine->check_item_get($groups->[0]->{location}); + + is_deeply($groups->[0]->{content}->{pbx_groupmember_ids}, $group_get_after->{pbx_groupmember_ids}, "Check group members order after touching it's member"); + + $test_machine->clear_test_data_all();#fake data aren't registered in this test machine, so they will stay. } }