D5556.1775281588.diff
No OneTemporary
Actions

Authored By

Unknown

Size

36 KB

Referenced Files

None

Subscribers

None

D5556.1775281588.diff
View Options

	diff --git a/docker/postfix/rootfs/etc/postfix/main.cf b/docker/postfix/rootfs/etc/postfix/main.cf
	--- a/docker/postfix/rootfs/etc/postfix/main.cf
	+++ b/docker/postfix/rootfs/etc/postfix/main.cf
	@@ -499,13 +499,20 @@
	meta_directory = /etc/postfix
	shlib_directory = /usr/lib64/postfix
	recipient_delimiter = +
	-local_recipient_maps = mysql:/etc/postfix/sql/local_recipient_maps.cf
	-#transport_maps = ldap:/etc/postfix/ldap/transport_maps.cf, hash:/etc/postfix/transport
	transport_maps = regexp:/etc/postfix/transport
	-virtual_alias_maps = $alias_maps, mysql:/etc/postfix/sql/virtual_alias_maps.cf
	-#Other virtual alias maps we used to have: ldap:/etc/postfix/ldap/virtual_alias_maps_mailforwarding.cf, ldap:/etc/postfix/ldap/virtual_alias_maps_sharedfolders.cf, ldap:/etc/postfix/ldap/mailenabled_distgroups.cf, ldap:/etc/postfix/ldap/mailenabled_dynamic_distgroups.cf
	smtpd_tls_auth_only = no

	+local_recipient_maps =
	+ mysql:/etc/postfix/sql/local_recipient_maps.cf,
	+ mysql:/etc/postfix/sql/local_recipient_maps_groups.cf,
	+ mysql:/etc/postfix/sql/local_recipient_maps_shared_folders.cf
	+
	+virtual_alias_maps =
	+ $alias_maps,
	+ mysql:/etc/postfix/sql/virtual_alias_maps.cf,
	+ mysql:/etc/postfix/sql/virtual_alias_maps_groups.cf,
	+ mysql:/etc/postfix/sql/virtual_alias_maps_shared_folders.cf
	+
	# Inbound
	smtpd_client_restrictions =
	permit_mynetworks,
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps.cf b/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps.cf
	--- a/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps.cf
	+++ b/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps.cf
	@@ -2,4 +2,5 @@
	user = DB_USERNAME
	password = DB_PASSWORD
	dbname = DB_DATABASE
	-query = SELECT users.email FROM users left join user_aliases ON users.id = user_aliases.user_id WHERE (users.email = '%u@%d' OR user_aliases.alias = '%u@%d') AND users.status & 2 AND NOT users.status & 4 AND NOT users.status & 8 LIMIT 1
	+query = SELECT u.email FROM users u LEFT JOIN user_aliases a ON u.id = a.user_id
	+ WHERE (u.email = '%u@%d' OR a.alias = '%u@%d') AND u.status & 2 AND NOT u.status & 4 AND u.deleted_at IS NULL
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps_groups.cf b/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps_groups.cf
	new file mode 100644
	--- /dev/null
	+++ b/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps_groups.cf
	@@ -0,0 +1,5 @@
	+hosts = DB_HOST
	+user = DB_USERNAME
	+password = DB_PASSWORD
	+dbname = DB_DATABASE
	+query = SELECT email FROM groups WHERE email = '%u@%d' AND status & 2 AND NOT status & 4 AND deleted_at IS NULL
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps_shared_folders.cf b/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps_shared_folders.cf
	new file mode 100644
	--- /dev/null
	+++ b/docker/postfix/rootfs/etc/postfix/sql/local_recipient_maps_shared_folders.cf
	@@ -0,0 +1,6 @@
	+hosts = DB_HOST
	+user = DB_USERNAME
	+password = DB_PASSWORD
	+dbname = DB_DATABASE
	+query = SELECT f.email FROM shared_folders f LEFT JOIN shared_folder_aliases a ON f.id = a.shared_folder_id
	+ WHERE (f.email = '%u@%d' OR a.alias = '%u@%d') AND f.type = 'mail' AND f.status & 2 AND NOT f.status & 4 AND f.deleted_at IS NULL
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/mydestination.cf b/docker/postfix/rootfs/etc/postfix/sql/mydestination.cf
	--- a/docker/postfix/rootfs/etc/postfix/sql/mydestination.cf
	+++ b/docker/postfix/rootfs/etc/postfix/sql/mydestination.cf
	@@ -2,4 +2,4 @@
	user = DB_USERNAME
	password = DB_PASSWORD
	dbname = DB_DATABASE
	-query = SELECT namespace FROM domains WHERE namespace = '%s' AND status & 2 AND status & 16 AND status & 32
	+query = SELECT namespace FROM domains WHERE namespace = '%s' AND status & (2 + 16 + 32) AND deleted_at IS NULL
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps.cf b/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps.cf
	--- a/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps.cf
	+++ b/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps.cf
	@@ -2,4 +2,5 @@
	user = DB_USERNAME
	password = DB_PASSWORD
	dbname = DB_DATABASE
	-query = SELECT users.email FROM users left join user_aliases ON users.id = user_aliases.user_id WHERE (users.email = '%s' OR user_aliases.alias = '%s') AND users.status & 2 AND NOT users.status & 4 AND NOT users.status & 8 LIMIT 1
	+query = SELECT u.email FROM users u LEFT JOIN user_aliases a ON u.id = a.user_id
	+ WHERE (u.email = '%s' OR a.alias = '%s') AND u.status & 2 AND NOT u.status & 4 AND u.deleted_at IS NULL
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps_groups.cf b/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps_groups.cf
	new file mode 100644
	--- /dev/null
	+++ b/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps_groups.cf
	@@ -0,0 +1,6 @@
	+hosts = DB_HOST
	+user = DB_USERNAME
	+password = DB_PASSWORD
	+dbname = DB_DATABASE
	+query = SELECT m.email FROM group_members m JOIN groups g ON g.id = m.group_id
	+ WHERE g.email = '%s' AND g.status & 2 AND NOT g.status & 4 AND g.deleted_at IS NULL
	diff --git a/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps_shared_folders.cf b/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps_shared_folders.cf
	new file mode 100644
	--- /dev/null
	+++ b/docker/postfix/rootfs/etc/postfix/sql/virtual_alias_maps_shared_folders.cf
	@@ -0,0 +1,6 @@
	+hosts = DB_HOST
	+user = DB_USERNAME
	+password = DB_PASSWORD
	+dbname = DB_DATABASE
	+query = SELECT f.email FROM shared_folders f LEFT JOIN shared_folder_aliases a ON f.id = a.shared_folder_id
	+ WHERE (f.email = '%s' OR a.alias = '%s') AND f.type = 'mail' AND f.status & 2 AND NOT f.status & 4 AND f.deleted_at IS NULL
	diff --git a/src/app/Backends/LDAP.php b/src/app/Backends/LDAP.php
	--- a/src/app/Backends/LDAP.php
	+++ b/src/app/Backends/LDAP.php
	@@ -947,8 +947,9 @@
	$groupDomain = explode('@', $group->email, 2)[1];
	$domainBaseDN = self::baseDN($ldap, $groupDomain);
	$validMembers = [];
	+ $members = $group->getAddresses();

	- foreach ($group->members as $member) {
	+ foreach ($members as $member) {
	[$local, $domainName] = explode('@', $member);

	$memberDN = "uid={$member},ou=People,{$domainBaseDN}";
	@@ -982,9 +983,8 @@

	// Update members in sql (some might have been removed),
	// skip model events to not invoke another update job
	- if ($group->members !== $validMembers) {
	- $group->members = $validMembers;
	- $group->saveQuietly();
	+ if ($members !== $validMembers) {
	+ $group->setAddresses($validMembers, true);
	}
	}

	diff --git a/src/app/Console/Commands/Data/Import/LdifCommand.php b/src/app/Console/Commands/Data/Import/LdifCommand.php
	--- a/src/app/Console/Commands/Data/Import/LdifCommand.php
	+++ b/src/app/Console/Commands/Data/Import/LdifCommand.php
	@@ -376,10 +376,10 @@
	$group = new Group();
	$group->name = $data->name;
	$group->email = $data->email;
	- $group->members = $members;
	$group->tenant_id = $this->tenantId;
	$group->save();

	+ $group->setAddresses($members, true);
	$group->assignToWallet($this->wallet);

	// Sender policy
	diff --git a/src/app/Console/Commands/Group/AddMemberCommand.php b/src/app/Console/Commands/Group/AddMemberCommand.php
	--- a/src/app/Console/Commands/Group/AddMemberCommand.php
	+++ b/src/app/Console/Commands/Group/AddMemberCommand.php
	@@ -37,7 +37,9 @@
	return 1;
	}

	- if (in_array($member, $group->members)) {
	+ $existing = $group->getAddresses();
	+
	+ if (in_array($member, $existing)) {
	$this->error("{$member}: Already exists in the group.");
	return 1;
	}
	@@ -49,8 +51,7 @@
	return 1;
	}

	- // We can't modify the property indirectly, therefor array_merge()
	- $group->members = array_merge($group->members, [$member]);
	- $group->save();
	+ $existing[] = $member;
	+ $group->setAddresses($existing);
	}
	}
	diff --git a/src/app/Console/Commands/Group/CreateCommand.php b/src/app/Console/Commands/Group/CreateCommand.php
	--- a/src/app/Console/Commands/Group/CreateCommand.php
	+++ b/src/app/Console/Commands/Group/CreateCommand.php
	@@ -77,10 +77,10 @@
	// Create the group
	$group = new Group();
	$group->email = $email;
	- $group->members = $members;
	$group->tenant_id = $domain->tenant_id;
	$group->save();

	+ $group->setAddresses($members, true);
	$group->assignToWallet($owner->wallets->first());

	DB::commit();
	diff --git a/src/app/Console/Commands/Group/InfoCommand.php b/src/app/Console/Commands/Group/InfoCommand.php
	--- a/src/app/Console/Commands/Group/InfoCommand.php
	+++ b/src/app/Console/Commands/Group/InfoCommand.php
	@@ -41,7 +41,7 @@

	// TODO: Print owner/wallet

	- foreach ($group->members as $member) {
	+ foreach ($group->getAddresses() as $member) {
	$this->info('Member: ' . $member);
	}
	}
	diff --git a/src/app/Console/Commands/Group/RemoveMemberCommand.php b/src/app/Console/Commands/Group/RemoveMemberCommand.php
	--- a/src/app/Console/Commands/Group/RemoveMemberCommand.php
	+++ b/src/app/Console/Commands/Group/RemoveMemberCommand.php
	@@ -37,19 +37,19 @@
	}

	$members = [];
	+ $existing = $group->getAddresses();

	- foreach ($group->members as $m) {
	+ foreach ($existing as $m) {
	if ($m !== $member) {
	$members[] = $m;
	}
	}

	- if (count($members) == count($group->members)) {
	+ if (count($members) == count($existing)) {
	$this->error("Member {$member} not found in the group.");
	return 1;
	}

	- $group->members = $members;
	- $group->save();
	+ $group->setAddresses($members);
	}
	}
	diff --git a/src/app/Group.php b/src/app/Group.php
	--- a/src/app/Group.php
	+++ b/src/app/Group.php
	@@ -10,6 +10,7 @@
	use App\Traits\StatusPropertyTrait;
	use App\Traits\UuidIntKeyTrait;
	use Illuminate\Database\Eloquent\Model;
	+use Illuminate\Database\Eloquent\Relations\HasMany;
	use Illuminate\Database\Eloquent\SoftDeletes;

	/**
	@@ -17,7 +18,6 @@
	*
	* @property int $id The group identifier
	* @property string $email An email address
	- * @property array $members A list of email addresses
	* @property string $name The group name
	* @property int $status The group status
	* @property int $tenant_id Tenant identifier
	@@ -61,34 +61,52 @@
	/** @var list<string> The attributes that are mass assignable */
	protected $fillable = [
	'email',
	- 'members',
	'name',
	'status',
	];

	/**
	- * Group members propert accessor. Converts internal comma-separated list into an array
	- *
	- * @param string $members Comma-separated list of email addresses
	- *
	- * @return array Email addresses of the group members, as an array
	+ * Returns list of group member email addresses.
	*/
	- public function getMembersAttribute($members): array
	+ public function getAddresses(): array
	{
	- return $members ? explode(',', $members) : [];
	+ return $this->members()->orderBy('email')->pluck('email')->all();
	}

	/**
	- * Ensure the members are appropriately formatted.
	+ * Replace members with a new list of members.
	*
	* @param array $members Email addresses of the group members
	*/
	- public function setMembersAttribute(array $members): void
	+ public function setAddresses(array $members, bool $silently = false): void
	{
	$members = array_unique(array_filter(array_map('strtolower', $members)));

	- sort($members);
	+ $existing = $this->getAddresses();
	+ $added = array_diff($members, $existing);
	+ $removed = array_diff($existing, $members);
	+
	+ if (count($removed)) {
	+ $this->members()->whereIn('email', $removed)->delete();
	+ }
	+
	+ if (count($added)) {
	+ $this->members()->createMany(array_map(fn ($member) => ['email' => $member], $added));
	+ }
	+
	+ if (!$silently && !$this->trashed() && (count($removed) \|\| count($added))) {
	+ // Trigger an update job on the group, as we do not observe members
	+ \App\Jobs\Group\UpdateJob::dispatch($this->id);
	+ }
	+ }

	- $this->attributes['members'] = implode(',', $members);
	+ /**
	+ * The relationship to members.
	+ *
	+ * @return HasMany<GroupMember, $this>
	+ */
	+ public function members()
	+ {
	+ return $this->hasMany(GroupMember::class);
	}
	}
	diff --git a/src/app/GroupMember.php b/src/app/GroupMember.php
	new file mode 100644
	--- /dev/null
	+++ b/src/app/GroupMember.php
	@@ -0,0 +1,42 @@
	+<?php
	+
	+namespace App;
	+
	+use Illuminate\Database\Eloquent\Model;
	+use Illuminate\Database\Eloquent\Relations\BelongsTo;
	+
	+/**
	+ * A collection of group members.
	+ *
	+ * @property int $id
	+ * @property int $group_id
	+ * @property string $email
	+ */
	+class GroupMember extends Model
	+{
	+ /** @var list<string> The attributes that are mass assignable */
	+ protected $fillable = ['group_id', 'email'];
	+
	+ /** @var bool Indicates if the model should be timestamped. */
	+ public $timestamps = false;
	+
	+ /**
	+ * The group to which this member belongs.
	+ *
	+ * @return BelongsTo<Group, $this>
	+ */
	+ public function group()
	+ {
	+ return $this->belongsTo(Group::class, 'group_id', 'id');
	+ }
	+
	+ /**
	+ * Ensure the email address is appropriately cased.
	+ *
	+ * @param string $email Email address
	+ */
	+ public function setEmailAttribute(string $email)
	+ {
	+ $this->attributes['email'] = \strtolower($email);
	+ }
	+}
	diff --git a/src/app/Http/Controllers/API/V4/GroupsController.php b/src/app/Http/Controllers/API/V4/GroupsController.php
	--- a/src/app/Http/Controllers/API/V4/GroupsController.php
	+++ b/src/app/Http/Controllers/API/V4/GroupsController.php
	@@ -37,6 +37,25 @@
	*/
	protected $deleteBeforeCreate;

	+ /**
	+ * Prepare a resource object for the UI.
	+ *
	+ * @param object $object An object
	+ * @param bool $full Include all object properties
	+ *
	+ * @return array Object information
	+ */
	+ protected function objectToClient($object, bool $full = false): array
	+ {
	+ $result = parent::objectToCLient($object, $full);
	+
	+ if ($full) {
	+ $result['members'] = $object->getAddresses();
	+ }
	+
	+ return $result;
	+ }
	+
	/**
	* Group status (extended) information
	*
	@@ -126,9 +145,9 @@
	$group = new Group();
	$group->name = $request->input('name');
	$group->email = $email;
	- $group->members = $members;
	$group->save();

	+ $group->setAddresses($members, true);
	$group->assignToWallet($wallet);

	DB::commit();
	@@ -203,8 +222,8 @@

	// SkusController::updateEntitlements($group, $request->skus);

	- $group->members = $members;
	$group->save();
	+ $group->setAddresses($members);

	return response()->json([
	'status' => 'success',
	diff --git a/src/app/Observers/UserObserver.php b/src/app/Observers/UserObserver.php
	--- a/src/app/Observers/UserObserver.php
	+++ b/src/app/Observers/UserObserver.php
	@@ -5,7 +5,7 @@
	use App\Delegation;
	use App\Entitlement;
	use App\EventLog;
	-use App\Group;
	+use App\GroupMember;
	use App\Jobs\PGP\KeyCreateJob;
	use App\Jobs\PGP\KeyDeleteJob;
	use App\Jobs\User\CreateJob;
	@@ -106,16 +106,13 @@
	}

	// Remove the user from existing groups
	- $wallet = $user->wallet();
	- if ($wallet && $wallet->owner) {
	- $wallet->owner->groups()->each(static function ($group) use ($user) {
	- /** @var Group $group */
	- if (in_array($user->email, $group->members)) {
	- $group->members = array_diff($group->members, [$user->email]);
	- $group->save();
	- }
	- });
	- }
	+ GroupMember::where('email', $user->email)->get()->each(static function ($member) {
	+ $member->delete();
	+ if ($member->group) {
	+ // Trigger an update job on the group, as we do not observe members
	+ \App\Jobs\Group\UpdateJob::dispatch($member->group->id);
	+ }
	+ });

	// Remove delegation relations
	$ids = Delegation::where('user_id', $user->id)->orWhere('delegatee_id', $user->id)->get()
	diff --git a/src/app/Policy/SmtpAccess.php b/src/app/Policy/SmtpAccess.php
	--- a/src/app/Policy/SmtpAccess.php
	+++ b/src/app/Policy/SmtpAccess.php
	@@ -52,10 +52,8 @@
	}

	// TODO: Prepending Sender/X-Sender/X-Authenticated-As headers?
	- // TODO: Recipient policies here?

	- // Leave it up to the postfix configuration how to proceed
	- // (accept would stop processing)
	+ // Leave it up to the postfix configuration how to proceed (accept would stop processing)
	return new Response(Response::ACTION_DUNNO);
	}

	@@ -72,8 +70,6 @@
	}

	// TODO: Make sure the domain is not suspended
	- // TODO: Email might belong to a group (distlists), check group's sender_policy
	- // TODO: Email might be a shared folder (or it's alias)?

	$email = \strtolower($email);

	diff --git a/src/database/migrations/2025_08_21_100000_create_group_members_table.php b/src/database/migrations/2025_08_21_100000_create_group_members_table.php
	new file mode 100644
	--- /dev/null
	+++ b/src/database/migrations/2025_08_21_100000_create_group_members_table.php
	@@ -0,0 +1,61 @@
	+<?php
	+
	+use App\Group;
	+use Illuminate\Database\Migrations\Migration;
	+use Illuminate\Database\Schema\Blueprint;
	+use Illuminate\Support\Facades\Schema;
	+
	+return new class extends Migration {
	+ /**
	+ * Run the migrations.
	+ */
	+ public function up(): void
	+ {
	+ Schema::create(
	+ 'group_members',
	+ static function (Blueprint $table) {
	+ $table->bigIncrements('id');
	+ $table->bigInteger('group_id');
	+ $table->string('email')->index();
	+ $table->timestamp('created_at')->useCurrent();
	+
	+ $table->unique(['group_id', 'email']);
	+
	+ $table->foreign('group_id')->references('id')->on('groups')
	+ ->onDelete('cascade')->onUpdate('cascade');
	+ }
	+ );
	+
	+ Group::select('id', 'members')->get()->each(function ($group) {
	+ $members = explode(',', $group->members);
	+ $group->setAddresses($members);
	+ });
	+
	+ Schema::table(
	+ 'groups',
	+ static function (Blueprint $table) {
	+ $table->dropColumn('members');
	+ }
	+ );
	+ }
	+
	+ /**
	+ * Reverse the migrations.
	+ */
	+ public function down(): void
	+ {
	+ Schema::table(
	+ 'groups',
	+ static function (Blueprint $table) {
	+ $table->text('members')->nullable();
	+ }
	+ );
	+
	+ Group::all()->each(function ($group) {
	+ $members = $group->getAddresses();
	+ DB::table('groups')->where('id', $group->id)->update(['members' => implode(',', $members)]);
	+ });
	+
	+ Schema::dropIfExists('group_members');
	+ }
	+};
	diff --git a/src/tests/Browser/Admin/DistlistTest.php b/src/tests/Browser/Admin/DistlistTest.php
	--- a/src/tests/Browser/Admin/DistlistTest.php
	+++ b/src/tests/Browser/Admin/DistlistTest.php
	@@ -60,8 +60,7 @@
	$user = $this->getTestUser('john@kolab.org');
	$group = $this->getTestGroup('group-test@kolab.org', ['name' => 'Test Group']);
	$group->assignToWallet($user->wallets->first());
	- $group->members = ['test1@gmail.com', 'test2@gmail.com'];
	- $group->save();
	+ $group->setAddresses(['test1@gmail.com', 'test2@gmail.com'], true);
	$group->setConfig(['sender_policy' => ['test1.com', 'test2.com']]);

	$event1 = EventLog::createFor($group, EventLog::TYPE_SUSPENDED, 'Event 1');
	@@ -85,6 +84,7 @@
	->on($distlist_page)
	->assertSeeIn('@distlist-info .card-title', $group->email)
	->with('@distlist-info form', static function (Browser $browser) use ($group) {
	+ $members = $group->getAddresses();
	$browser->assertElementsCount('.row', 4)
	->assertSeeIn('.row:nth-child(1) label', 'ID (Created)')
	->assertSeeIn('.row:nth-child(1) #distlistid', "{$group->id} ({$group->created_at})")
	@@ -93,8 +93,8 @@
	->assertSeeIn('.row:nth-child(3) label', 'Name')
	->assertSeeIn('.row:nth-child(3) #name', $group->name)
	->assertSeeIn('.row:nth-child(4) label', 'Recipients')
	- ->assertSeeIn('.row:nth-child(4) #members', $group->members[0])
	- ->assertSeeIn('.row:nth-child(4) #members', $group->members[1]);
	+ ->assertSeeIn('.row:nth-child(4) #members', $members[0])
	+ ->assertSeeIn('.row:nth-child(4) #members', $members[1]);
	})
	->assertElementsCount('ul.nav-tabs li', 2)
	->assertSeeIn('ul.nav-tabs #tab-settings', 'Settings')
	diff --git a/src/tests/Browser/DistlistTest.php b/src/tests/Browser/DistlistTest.php
	--- a/src/tests/Browser/DistlistTest.php
	+++ b/src/tests/Browser/DistlistTest.php
	@@ -201,7 +201,7 @@
	->assertElementsCount('@table tbody tr', 1);

	$group = Group::where('email', 'group-test@kolab.org')->first();
	- $this->assertSame(['test1@gmail.com'], $group->members);
	+ $this->assertSame(['test1@gmail.com'], $group->getAddresses());

	// Test group deletion
	$browser->click('@table tr:nth-child(1) td:first-child a')
	diff --git a/src/tests/Browser/Reseller/DistlistTest.php b/src/tests/Browser/Reseller/DistlistTest.php
	--- a/src/tests/Browser/Reseller/DistlistTest.php
	+++ b/src/tests/Browser/Reseller/DistlistTest.php
	@@ -60,8 +60,7 @@
	$user = $this->getTestUser('john@kolab.org');
	$group = $this->getTestGroup('group-test@kolab.org', ['name' => 'Test Group']);
	$group->assignToWallet($user->wallets->first());
	- $group->members = ['test1@gmail.com', 'test2@gmail.com'];
	- $group->save();
	+ $group->setAddresses(['test1@gmail.com', 'test2@gmail.com'], true);
	$group->setConfig(['sender_policy' => ['test1.com', 'test2.com']]);

	$distlist_page = new DistlistPage($group->id);
	@@ -79,6 +78,7 @@
	->on($distlist_page)
	->assertSeeIn('@distlist-info .card-title', $group->email)
	->with('@distlist-info form', static function (Browser $browser) use ($group) {
	+ $members = $group->getAddresses();
	$browser->assertElementsCount('.row', 4)
	->assertSeeIn('.row:nth-child(1) label', 'ID (Created)')
	->assertSeeIn('.row:nth-child(1) #distlistid', "{$group->id} ({$group->created_at})")
	@@ -87,8 +87,8 @@
	->assertSeeIn('.row:nth-child(3) label', 'Name')
	->assertSeeIn('.row:nth-child(3) #name', $group->name)
	->assertSeeIn('.row:nth-child(4) label', 'Recipients')
	- ->assertSeeIn('.row:nth-child(4) #members', $group->members[0])
	- ->assertSeeIn('.row:nth-child(4) #members', $group->members[1]);
	+ ->assertSeeIn('.row:nth-child(4) #members', $members[0])
	+ ->assertSeeIn('.row:nth-child(4) #members', $members[1]);
	})
	->assertElementsCount('ul.nav-tabs li', 2)
	->assertSeeIn('ul.nav-tabs #tab-settings', 'Settings')
	diff --git a/src/tests/Feature/Backends/LDAPTest.php b/src/tests/Feature/Backends/LDAPTest.php
	--- a/src/tests/Feature/Backends/LDAPTest.php
	+++ b/src/tests/Feature/Backends/LDAPTest.php
	@@ -162,8 +162,7 @@
	}

	// Update members
	- $group->members = ['member3@testldap.com'];
	- $group->save();
	+ $group->setAddresses(['member3@testldap.com'], true);
	$group->setSetting('sender_policy', '["test.com","Test.com","-"]');

	LDAP::updateGroup($group);
	@@ -178,11 +177,11 @@
	$this->assertSame($value, $ldap_group[$attr] ?? null, "Group {$attr} attribute");
	}

	- $this->assertSame(['member3@testldap.com'], $group->fresh()->members);
	+ $this->assertSame(['member3@testldap.com'], $group->getAddresses());

	// Update members (add non-existing local member, expect it to be aot-removed from the group)
	// Update group name and sender_policy
	- $group->members = ['member3@testldap.com', 'member-local@kolab.org'];
	+ $group->setAddresses(['member3@testldap.com', 'member-local@kolab.org'], true);
	$group->name = 'Te(=ść)1';
	$group->save();
	$group->setSetting('sender_policy', null);
	@@ -201,7 +200,7 @@
	$this->assertSame($value, $ldap_group[$attr] ?? null, "Group {$attr} attribute");
	}

	- $this->assertSame(['member3@testldap.com'], $group->fresh()->members);
	+ $this->assertSame(['member3@testldap.com'], $group->getAddresses());

	// We called save() twice, and setSettings() three times,
	// this is making sure that there's no job executed by the LDAP backend
	diff --git a/src/tests/Feature/Console/Data/Import/LdifTest.php b/src/tests/Feature/Console/Data/Import/LdifTest.php
	--- a/src/tests/Feature/Console/Data/Import/LdifTest.php
	+++ b/src/tests/Feature/Console/Data/Import/LdifTest.php
	@@ -172,7 +172,7 @@
	$this->assertCount(1, $groups);
	$this->assertSame('Group', $groups[0]->name);
	$this->assertSame('group@kolab3.com', $groups[0]->email);
	- $this->assertSame(['owner@kolab3.com', 'user@kolab3.com'], $groups[0]->members);
	+ $this->assertSame(['owner@kolab3.com', 'user@kolab3.com'], $groups[0]->getAddresses());
	$this->assertSame('["sender@gmail.com","-"]', $groups[0]->getSetting('sender_policy'));

	// Resources
	diff --git a/src/tests/Feature/Console/Group/AddMemberTest.php b/src/tests/Feature/Console/Group/AddMemberTest.php
	--- a/src/tests/Feature/Console/Group/AddMemberTest.php
	+++ b/src/tests/Feature/Console/Group/AddMemberTest.php
	@@ -58,7 +58,7 @@

	$this->assertSame(0, $code);
	$this->assertSame('', $output);
	- $this->assertSame(['member@gmail.com'], $group->refresh()->members);
	+ $this->assertSame(['member@gmail.com'], $group->getAddresses());

	// Existing group
	$code = \Artisan::call("group:add-member {$group->email} member2@gmail.com");
	@@ -66,7 +66,7 @@

	$this->assertSame(0, $code);
	$this->assertSame('', $output);
	- $this->assertSame(['member2@gmail.com', 'member@gmail.com'], $group->refresh()->members);
	+ $this->assertSame(['member@gmail.com', 'member2@gmail.com'], $group->getAddresses());

	// Add a member that already exists
	$code = \Artisan::call("group:add-member {$group->email} member@gmail.com");
	@@ -74,7 +74,7 @@

	$this->assertSame(1, $code);
	$this->assertSame("member@gmail.com: Already exists in the group.", $output);
	- $this->assertSame(['member2@gmail.com', 'member@gmail.com'], $group->refresh()->members);
	+ $this->assertSame(['member@gmail.com', 'member2@gmail.com'], $group->getAddresses());

	// Adding a local-domain member that does not exist
	$john = $this->getTestUser('john@kolab.org');
	@@ -86,6 +86,6 @@

	$this->assertSame(1, $code);
	$this->assertSame("member-unknown@kolab.org: The specified email address does not exist.", $output);
	- $this->assertSame([], $group->refresh()->members);
	+ $this->assertSame([], $group->getAddresses());
	}
	}
	diff --git a/src/tests/Feature/Console/Group/CreateTest.php b/src/tests/Feature/Console/Group/CreateTest.php
	--- a/src/tests/Feature/Console/Group/CreateTest.php
	+++ b/src/tests/Feature/Console/Group/CreateTest.php
	@@ -67,7 +67,7 @@

	$this->assertSame(0, $code);
	$this->assertSame((string) $group->id, $output);
	- $this->assertSame([], $group->members);
	+ $this->assertSame([], $group->getAddresses());
	$this->assertSame($user->wallets->first()->id, $group->wallet()->id);

	// Existing email (of a group)
	@@ -90,7 +90,7 @@
	$group = Group::where('email', 'group-testm@kolab.org')->first();
	$this->assertSame(0, $code);
	$this->assertSame((string) $group->id, $output);
	- $this->assertSame(['member1@kolabnow.com', 'member2@gmail.com'], $group->members);
	+ $this->assertSame(['member1@kolabnow.com', 'member2@gmail.com'], $group->getAddresses());
	$this->assertSame($user->wallets->first()->id, $group->wallet()->id);
	}
	}
	diff --git a/src/tests/Feature/Console/Group/InfoTest.php b/src/tests/Feature/Console/Group/InfoTest.php
	--- a/src/tests/Feature/Console/Group/InfoTest.php
	+++ b/src/tests/Feature/Console/Group/InfoTest.php
	@@ -45,8 +45,7 @@
	$this->assertSame($expected, $output);

	// Group with members
	- $group->members = ['test@member.com'];
	- $group->save();
	+ $group->setAddresses(['test@member.com']);

	$expected .= "\nMember: test@member.com";

	diff --git a/src/tests/Feature/Console/Group/RemoveMemberTest.php b/src/tests/Feature/Console/Group/RemoveMemberTest.php
	--- a/src/tests/Feature/Console/Group/RemoveMemberTest.php
	+++ b/src/tests/Feature/Console/Group/RemoveMemberTest.php
	@@ -39,10 +39,8 @@
	$this->assertSame(1, $code);
	$this->assertSame("Group test@group.com does not exist.", $output);

	- $group = Group::create([
	- 'email' => 'group-test@kolabnow.com',
	- 'members' => ['member1@gmail.com', 'member2@gmail.com'],
	- ]);
	+ $group = Group::create(['email' => 'group-test@kolabnow.com']);
	+ $group->setAddresses(['member1@gmail.com', 'member2@gmail.com']);

	// Existing group, non-existing member
	$code = \Artisan::call("group:remove-member {$group->email} nonexisting@gmail.com");
	@@ -57,7 +55,7 @@

	$this->assertSame(0, $code);
	$this->assertSame('', $output);
	- $this->assertSame(['member2@gmail.com'], $group->refresh()->members);
	+ $this->assertSame(['member2@gmail.com'], $group->getAddresses());

	// Existing group, the last existing member
	$code = \Artisan::call("group:remove-member {$group->email} member2@gmail.com");
	@@ -65,6 +63,6 @@

	$this->assertSame(0, $code);
	$this->assertSame('', $output);
	- $this->assertSame([], $group->refresh()->members);
	+ $this->assertSame([], $group->getAddresses());
	}
	}
	diff --git a/src/tests/Feature/Controller/DomainsTest.php b/src/tests/Feature/Controller/DomainsTest.php
	--- a/src/tests/Feature/Controller/DomainsTest.php
	+++ b/src/tests/Feature/Controller/DomainsTest.php
	@@ -340,7 +340,7 @@
	$this->assertSame([], $json['config']['spf_whitelist']);
	$this->assertCount(4, $json['mx']);
	$this->assertTrue(str_contains(implode("\n", $json['mx']), $domain->namespace));
	- $this->assertCount(8, $json['dns']);
	+ $this->assertCount(1, $json['dns']);
	$this->assertTrue(str_contains(implode("\n", $json['dns']), $domain->namespace));
	$this->assertTrue(str_contains(implode("\n", $json['dns']), $domain->hash()));
	$this->assertTrue(is_array($json['statusInfo']));
	diff --git a/src/tests/Feature/Controller/GroupsTest.php b/src/tests/Feature/Controller/GroupsTest.php
	--- a/src/tests/Feature/Controller/GroupsTest.php
	+++ b/src/tests/Feature/Controller/GroupsTest.php
	@@ -245,7 +245,7 @@
	$this->assertSame($group->id, $json['id']);
	$this->assertSame($group->email, $json['email']);
	$this->assertSame($group->name, $json['name']);
	- $this->assertSame($group->members, $json['members']);
	+ $this->assertSame([], $json['members']);
	$this->assertTrue(!empty($json['statusInfo']));
	$this->assertArrayHasKey('isDeleted', $json);
	$this->assertArrayHasKey('isSuspended', $json);
	@@ -507,7 +507,7 @@
	$group = Group::where('email', 'group-test@kolab.org')->first();
	$this->assertInstanceOf(Group::class, $group);
	$this->assertSame($post['email'], $group->email);
	- $this->assertSame($post['members'], $group->members);
	+ $this->assertSame($post['members'], $group->getAddresses());
	$this->assertTrue($john->groups()->get()->contains($group));

	// Group name must be unique within a domain
	@@ -619,7 +619,7 @@
	$group->refresh();

	$this->assertSame($post['name'], $group->name);
	- $this->assertSame($post['members'], $group->members);
	+ $this->assertSame($post['members'], $group->getAddresses());
	}

	/**
	diff --git a/src/tests/Feature/GroupTest.php b/src/tests/Feature/GroupTest.php
	--- a/src/tests/Feature/GroupTest.php
	+++ b/src/tests/Feature/GroupTest.php
	@@ -30,6 +30,47 @@
	parent::tearDown();
	}

	+ /**
	+ * Test Group::addAddresses()
	+ */
	+ public function testAddAddresses(): void
	+ {
	+ Queue::fake();
	+
	+ $group = $this->getTestGroup('group-test@kolabnow.com');
	+
	+ $group->setAddresses(['test@gmail.com']);
	+
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1);
	+ $this->assertSame(['test@gmail.com'], $group->getAddresses());
	+
	+ $group->setAddresses(['test@gmail.com']);
	+
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1);
	+ $this->assertSame(['test@gmail.com'], $group->getAddresses());
	+
	+ $group->setAddresses([]);
	+
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 2);
	+ $this->assertSame([], $group->getAddresses());
	+
	+ $group->setAddresses(['test1@gmail.com', 'test2@gmail.com'], true);
	+
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 2);
	+ $this->assertSame(['test1@gmail.com', 'test2@gmail.com'], $group->getAddresses());
	+
	+ $group->setAddresses(['test1@gmail.com', 'test3@gmail.com']);
	+
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 3);
	+ $this->assertSame(['test1@gmail.com', 'test3@gmail.com'], $group->getAddresses());
	+
	+ $group->delete();
	+ $group->setAddresses(['test1@gmail.com']);
	+
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 3);
	+ $this->assertSame(['test1@gmail.com'], $group->getAddresses());
	+ }
	+
	/**
	* Tests for Group::assignToWallet()
	*/
	@@ -84,7 +125,6 @@
	$this->assertSame('group-test@kolabnow.com', $group->email);
	$this->assertSame('group-test', $group->name);
	$this->assertMatchesRegularExpression('/^[0-9]{1,20}$/', (string) $group->id);
	- $this->assertSame([], $group->members);
	$this->assertTrue($group->isNew());
	$this->assertFalse($group->isActive());

	diff --git a/src/tests/Feature/Jobs/User/CreateTest.php b/src/tests/Feature/Jobs/User/CreateTest.php
	--- a/src/tests/Feature/Jobs/User/CreateTest.php
	+++ b/src/tests/Feature/Jobs/User/CreateTest.php
	@@ -67,7 +67,7 @@

	// Test mailbox exists case
	$user->status ^= User::STATUS_IMAP_READY;
	- $user->save();
	+ $user->saveQuietly();
	IMAP::shouldReceive('createUser')->once()->with($user)->andThrow(new MailboxExistsException());

	$job = (new CreateJob($user->id))->withFakeQueueInteractions();
	diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php
	--- a/src/tests/Feature/UserTest.php
	+++ b/src/tests/Feature/UserTest.php
	@@ -965,22 +965,16 @@
	$userB = $this->getTestUser('UserAccountB@UserAccount.com');
	$userA->assignPackage($package_kolab, $userB);
	$group = $this->getTestGroup('test-group@UserAccount.com');
	- $group->members = ['test@gmail.com', $userB->email];
	+ $group->setAddresses(['test@gmail.com', $userB->email], true);
	$group->assignToWallet($userA->wallets->first());
	- $group->save();

	- Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1);
	-
	- $userGroups = $userA->groups()->get();
	- $this->assertSame(1, $userGroups->count());
	- $this->assertSame($group->id, $userGroups->first()->id);
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 0);

	$userB->delete();

	- $this->assertSame(['test@gmail.com'], $group->fresh()->members);
	+ $this->assertSame(['test@gmail.com'], $group->getAddresses());

	- // Twice, one for save() and one for delete() above
	- Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 2);
	+ Queue::assertPushed(\App\Jobs\Group\UpdateJob::class, 1);
	}

	/**

File Metadata

Mime Type: text/plain
Expires: Sat, Apr 4, 5:46 AM (13 h, 3 m)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 18824619
Default Alt Text: D5556.1775281588.diff (36 KB)

D5556.1775281588.diffNo OneTemporaryActions

D5556.1775281588.diffView Options

File Metadata

Event Timeline

D5556.1775281588.diff
No OneTemporary
Actions

D5556.1775281588.diff
View Options