spatie / mailcoach-support

Questions and support for Mailcoach
https://mailcoach.app
31 stars 2 forks source link

$emailList->subscribers and $emailList->allSubscribers return subscribers in random order #242

Closed connecteev closed 4 years ago

connecteev commented 4 years ago

See https://mailcoach.app/docs/v2/package/working-with-lists/subscribing-to-a-list

$emailList->subscribers and $emailList->allSubscribers returns subscribers in random order

This returns subscribers in random order, so $emailList1->subscribers->first()->email and $emailList1->subscribers->last()->email arent in the order they were added to the list

Similarly for $emailList->allSubscribers

freekmurze commented 4 years ago

Why do you think they are returned in a random order? We do nothing special regarding the order.

connecteev commented 4 years ago

This is not a blocker, but I reported it anyway.

Here's a unit test that should pass but doesn't:

    private $emailLists = [
        'list_1' => [
            'name' => 'My Email List #1',
            'requires_confirmation' => false,
            'default_from_email' => 'user1@keenbrain.com',
            'default_from_name' => 'User1 at Keenbrain',
        ],
        'list_2' => [
            'name' => 'My Email List #2',
            'requires_confirmation' => false,
            'default_from_email' => 'user2@keenbrain.com',
            'default_from_name' => 'User2 at Keenbrain',
        ],
        'list_3' => [
            'name' => 'My Email List #3',
            'requires_confirmation' => true,
            'default_from_email' => 'user3@keenbrain.com',
            'default_from_name' => 'User3 at Keenbrain',
        ],
    ];
    private $email_subscribers = [
        'person_1' => [
            'email' => 'john@person1.com',
            'details' => [
                'first_name' => 'John',
                'last_name' => 'Appleseed',
                'extra_attributes' => [
                    'key 1' => 'Value 1',
                    'key 2' => 'Value 2'
                ],
            ],
        ],
        'person_2' => [
            'email' => 'sam@person2.com',
            'details' => [
                'first_name' => 'Sam',
                'last_name' => 'Altman',
                'extra_attributes' => [
                    'userId' => 54,
                    'something' => 'perfect',
                ],
            ],
        ],
        'person_3' => [
            'email' => 'ben@person3.com',
            'details' => [
                'first_name' => 'Ben',
                'last_name' => 'Settle',
                'extra_attributes' => [
                    'hello' => 'there',
                ],
            ],
        ],
        'person_4' => [
            'email' => 'george@person4.com',
            'details' => [
                'first_name' => 'George',
                'last_name' => 'Kostanza',
                'extra_attributes' => [
                    'id' => 911,
                    'isCustomer' => true
                ],
            ],
        ],
        'person_5' => [
            'email' => 'tim@person5.com',
            'details' => [
                'first_name' => 'Tim',
                'last_name' => 'Worthington',
                'extra_attributes' => [
                    'id' => 77,
                    'isCustomer' => false
                ],
            ],
        ],
    ];

    private function create_mailcoach_email_list($emailListParams)
    {
        // $emailList = EmailList::create($emailListParams);
        $emailList = EmailList::create([
            'name' => $emailListParams['name'], // list name is required

            // All of the following are optional params

            // Enable or Disable double opt-in confirmation
            'requires_confirmation' => $emailListParams['requires_confirmation'],

            // When sending a campaign to this email list these defaults will be used if you send a campaign that doesn't have a sender of its own.
            // As per https://mailcoach.app/docs/v2/package/working-with-lists/creating-a-list#setting-a-default-sender
            'default_from_email' => $emailListParams['default_from_email'],
            'default_from_name' => $emailListParams['default_from_name'],

            // Specify the mailers to be used for this email list
            // As per https://mailcoach.app/docs/v2/package/working-with-lists/creating-a-list#specifing-the-mailers-to-be-used
            'campaign_mailer' => env('MAILCOACH_CAMPAIGN_MAILER'),
            'transactional_mailer' => env('MAILCOACH_TRANSACTIONAL_MAILER'),
        ]);

        $this->assertNotNull($emailList);
        $this->assertEquals($emailList['name'], $emailListParams['name']);
        $this->assertEquals($emailList['requires_confirmation'], $emailListParams['requires_confirmation']);
        $this->assertEquals($emailList['default_from_email'], $emailListParams['default_from_email']);
        $this->assertEquals($emailList['default_from_name'], $emailListParams['default_from_name']);
        return $emailList;
    }

    public function test_can_get_email_list_subscribers_in_correct_order()
    {
        // Create Mailcoach email list (with double opt-in confirmation DISABLED)
        $emailList1 = $this->create_mailcoach_email_list($this->emailLists['list_1']);

        $dataSubscriber1 = $this->email_subscribers['person_1'];
        $emailList1->subscribe($dataSubscriber1['email'], $dataSubscriber1['details']);
        $this->assertEquals(1, Subscriber::where(['email_list_id' => $emailList1->id])->count());

        sleep(2);
        $dataSubscriber2 = $this->email_subscribers['person_2'];
        $emailList1->subscribe($dataSubscriber2['email'], $dataSubscriber2['details']);
        $this->assertEquals(2, Subscriber::where(['email_list_id' => $emailList1->id])->count());

        sleep(2);
        $dataSubscriber3 = $this->email_subscribers['person_3'];
        $emailList1->subscribe($dataSubscriber3['email'], $dataSubscriber3['details']);
        $this->assertEquals(3, Subscriber::where(['email_list_id' => $emailList1->id])->count());

        sleep(2);
        $dataSubscriber4 = $this->email_subscribers['person_4'];
        $emailList1->subscribe($dataSubscriber4['email'], $dataSubscriber4['details']);
        $this->assertEquals(4, Subscriber::where(['email_list_id' => $emailList1->id])->count());

        sleep(2);
        $dataSubscriber5 = $this->email_subscribers['person_5'];
        $emailList1->subscribe($dataSubscriber5['email'], $dataSubscriber5['details']);
        $this->assertEquals(5, Subscriber::where(['email_list_id' => $emailList1->id])->count());

        // unsubscribing the email changes the status to 'unsubscribed', but does not remove the entry from the email list
        $dataSubscriber5 = $this->email_subscribers['person_5'];
        $emailList1->unsubscribe($dataSubscriber5['email']);
        $this->assertEquals(5, Subscriber::where(['email_list_id' => $emailList1->id])->count());

        // Per https://mailcoach.app/docs/v2/package/working-with-lists/subscribing-to-a-list#getting-all-list-subscribers
        $this->assertEquals(4, $emailList1->subscribers->count()); // To get all subscribers of an email list
        $this->assertEquals(5, $emailList1->allSubscribers->count()); // To get all subscribers, including all unconfirmed and unsubscribed ones

        // dd($emailList1->subscribers->pluck('email'));

        $this->assertEquals($this->email_subscribers['person_1']['email'], $emailList1->subscribers->first()->email);
        $this->assertEquals($this->email_subscribers['person_4']['email'], $emailList1->subscribers->last()->email);
    }

If you enable the dd() command, you get:

Illuminate\Support\Collection^ {#7268
  #items: array:4 [
    0 => "ben@person3.com"
    1 => "george@person4.com"
    2 => "john@person1.com"
    3 => "sam@person2.com"
  ]
}
freekmurze commented 4 years ago

Try this:

$emailList->allSubscribers()->orderBy($fieldYouWantToOrderOn)->get();
connecteev commented 4 years ago

That works..thanks. It would also make sense to have these:

$emailList->subscribers
$emailList->subscribers()
$emailList->allSubscribers
$emailList->allSubscribers()

order by some field (whether that is id or date) by default...instead of what it currently is doing. But I leave that to your discretion.