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

Add credit/debit scopes to Line #192

Merged
merged 1 commit into from
Nov 23, 2021
Merged

Conversation

swrobel
Copy link
Contributor

@swrobel swrobel commented Aug 10, 2020

I've overridden locally in the past & found these scopes useful, so I thought they might be a welcome addition to 2.0

@ricobl
Copy link
Contributor

ricobl commented Nov 23, 2021

Hey @swrobel, thanks for the contribution! I'll merge this shortly and we'll bundle this up with other changes before issuing a release.

In the meantime, would you mind sharing a bit of your use case for this?

The migration doesn't include an index for the amount on the lines table, did you add an index on your app? Or is it querying against a subset of records?

@ricobl ricobl merged commit ea269e5 into envato:master Nov 23, 2021
@swrobel
Copy link
Contributor Author

swrobel commented Nov 23, 2021

I'm always using it against a subset of records, so I didn't think adding another index would be useful, but I suppose maybe there are some cases for others where it could be?

@ricobl
Copy link
Contributor

ricobl commented Nov 23, 2021

Fair enough! Thanks for sharing.

I guess the best approach is leaving the index out for now. Anyone with performance issues using those scopes can add the index to their app. In our experience the lines table can get pretty big so it's best to avoid non essential indexes.

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.

3 participants