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

fix: Make FakeContentTableSeeder runnable several times #2099

Merged
merged 7 commits into from Dec 9, 2018

Conversation

Projects
None yet
3 participants
@AdrienPoupa
Copy link
Contributor

AdrienPoupa commented Nov 26, 2018

I wanted to run the FakeContentTableSeeder several times but I figured it would only work once with a fresh DB. That is because it is first creating a new account, adding the contacts, then creating a second account.

When relaunching it, it would randomly get a ContactFieldType instance, that could belong to account number 2 whereas we run the seeder as account number 1. Then, the following failure is triggered:

ContactFieldType::where('account_id', $data['account_id'])
->findOrFail($data['contact_field_type_id']);

because it tries to access a ContactFieldType owned by account 2 whereas it queries account_id=1.

The fix I propose scopes the request to the current account and only creates the account admin@admin.com if it does not exist yet.

As a result, one can now run FakeContentTableSeeder more than once regardless of the DB state.

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Nov 26, 2018

CLA assistant check
All committers have signed the CLA.

AdrienPoupa added some commits Nov 26, 2018

@AdrienPoupa AdrienPoupa changed the title Make FakeContentTableSeeder runnable several times fix: Make FakeContentTableSeeder runnable several times Nov 26, 2018

AdrienPoupa added some commits Nov 27, 2018

@djaiss

This comment has been minimized.

Copy link
Member

djaiss commented Nov 27, 2018

Thanks for your help @AdrienPoupa
But the question is: why would you want to run it several times 😮 ?

@AdrienPoupa

This comment has been minimized.

Copy link
Contributor

AdrienPoupa commented Nov 27, 2018

Hi,
Say you want to insert 10 random contacts, then 10 others... Seeders should be runnable several times imo.
Now, I have a special use case where I want to study the behavior of Monica with different set of data and a growing number of records. I wanted to insert 20000 records (people) and it is not possible to do it all at once since the random user api is limited to 5000 entries. I discovered a loading issue on the people page and I'll do my best to fix it (ie paginate it). Thanks!

AdrienPoupa and others added some commits Dec 5, 2018

@djaiss

This comment has been minimized.

Copy link
Member

djaiss commented Dec 9, 2018

I’ll merge it now. Awesome work.

@djaiss djaiss merged commit c4a00eb into monicahq:master Dec 9, 2018

9 checks passed

Semantic Pull Request ready to be squashed
Details
SonarCloud Code Quality check passed
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: psalm Your tests passed on CircleCI!
Details
ci/circleci: reporting Your tests passed on CircleCI!
Details
ci/circleci: tests-7.2 Your tests passed on CircleCI!
Details
ci/circleci: tests-browser-7.2 Your tests passed on CircleCI!
Details
continuous-integration/styleci/pr The analysis has passed
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment