Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix - PostgreSQL Subscriber Growth Chart #15

Merged
merged 8 commits into from
Jun 10, 2020

Conversation

MaizerGomes
Copy link
Contributor

After installing using postgresql database, an error showed up informing that "date_format function doesn't exist".
The problem is in this file:
vendor/mettle/sendportal-core/src/Repositories/SubscriberTenantRepository.php

From line 152 to 166:
` $runningTotal = DB::table('subscribers')
->selectRaw("date_format(created_at, '%d-%m-%Y') AS date, count(*) as total")
->where('workspace_id', $workspaceId)
->where('created_at', '>=', $period->getStartDate())
->where('created_at', '<=', $period->getEndDate())
->groupBy('date')
->get();

$unsubscribers = DB::table('subscribers')
    ->selectRaw("date_format(unsubscribed_at, '%d-%m-%Y') AS date, count(*) as total")
    ->where('workspace_id', $workspaceId)
    ->where('unsubscribed_at', '>=', $period->getStartDate())
    ->where('unsubscribed_at', '<=', $period->getEndDate())
    ->groupBy('date')
    ->get();`

On postgresql you can use the function "to_char" instead of "date_format" since it doesn't exist:
->selectRaw("to_char(created_at, 'dd-mm-yyyy') AS date

Issue referenced here

@JonoB
Copy link
Contributor

JonoB commented Jun 10, 2020

HI @MaizerGomes - thanks for the PR!

We actually need a slightly different implementation to handle this. If you look at how we handle Campaigns (https://github.com/mettle/sendportal-core/tree/master/src/Repositories/Campaigns), you'll see that we have a Base repository, and then separate repositories for Mysql and Postgres.

We should do the same thing for Subscribers in this case. Do you want to have a try again?

@MaizerGomes MaizerGomes changed the title Check database driver before using date_format function (PostgreSQL fix) Extract SubscriberTenantRepository to an Interface fixing PostgreSQL issue - Edited Jun 10, 2020
@MaizerGomes
Copy link
Contributor Author

HI @MaizerGomes - thanks for the PR!

We actually need a slightly different implementation to handle this. If you look at how we handle Campaigns (https://github.com/mettle/sendportal-core/tree/master/src/Repositories/Campaigns), you'll see that we have a Base repository, and then separate repositories for Mysql and Postgres.

We should do the same thing for Subscribers in this case. Do you want to have a try again?

Thank you for your reply.
I took your advice and made the proper changes.

I still hadn't gone through the rest of the code and just wanted something quick just so I could try your app.

You're right, this approach is way cleaner and maintainable.

Copy link
Contributor

@willselby willselby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@joshuafranks
Copy link
Contributor

Looks great @MaizerGomes, thanks for the PR! 🎉

@joshuafranks joshuafranks changed the title Extract SubscriberTenantRepository to an Interface fixing PostgreSQL issue - Edited Bugfix - PostgreSQL Subscriber Growth Chart Jun 10, 2020
@joshuafranks joshuafranks merged commit 5937071 into mettle:master Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants