Add ordered seeds feature #1244

norkus256 · 2017-11-26T11:19:39Z

This feature is for ordering seeds execution by seeds dependencies. You can define other seeds which are dependencies for your seed and change seeds execution order automatically.

This feature is useful when you extend phinx manager (for example in unit tests to use in memory database) and do not have possibility execute seeds command from terminal.

Also if you have a lot of seeds it is not convenient to order seeds writing one by one in terminal to run ordered seeds.

Just run phinx seeds:run and your seeds will be executed in ordered way.

Use this function to define dependencies:

public function getDependencies()
{
    return [
        'UserSeeder',
        'GSeeder',
    ];
}

stickler-ci · 2017-11-26T11:20:00Z

src/Phinx/Migration/Manager.php

+    /**
+     * Get seed dependencies instances from seed dependency array
+     *
+     * @param OrderedSeedInterface $seed


Missing parameter comment

stickler-ci · 2017-11-26T11:20:00Z

src/Phinx/Migration/Manager.php

+                }
+            }
+        }
+        return $dependenciesInstances;


Missing blank line before return statement

stickler-ci · 2017-11-26T11:20:00Z

src/Phinx/Migration/Manager.php

+    /**
+     * Order seeds by dependencies
+     *
+     * @param AbstractSeed[] $seeds


Missing parameter comment

Also FQCN please inside comments.

stickler-ci · 2017-11-26T11:20:00Z

src/Phinx/Migration/Manager.php

+                $orderedSeeds[$key] = $seed;
+            }
+        }
+        return $orderedSeeds;


Missing blank line before return statement

codecov-io · 2017-11-26T11:24:01Z

Codecov Report

Merging #1244 into master will increase coverage by 0.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1244      +/-   ##
==========================================
+ Coverage   74.64%   74.76%   +0.12%     
==========================================
  Files          35       35              
  Lines        4741     4764      +23     
==========================================
+ Hits         3539     3562      +23     
  Misses       1202     1202

Impacted Files	Coverage Δ
src/Phinx/Seed/AbstractSeed.php	`48.83% <100%> (+2.49%)`	⬆️
src/Phinx/Migration/Manager.php	`90.54% <100%> (+0.52%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7072a54...00ffb18. Read the comment docs.

chinpei215 · 2017-12-10T05:53:37Z

src/Phinx/Migration/Manager.php

+        $dependencies = $seed->getDependencies();
+        foreach ($dependencies as $dependency) {
+            foreach ($this->seeds as $seed) {
+                if (get_class($seed) === $dependency) {


Sorry if I am wrong, but if people make a typo in the etDependencies() method, it will be discarded silently. Couldn't it be implemented by using usort() and the callback method? I think you want to sort seeds by a custom logic. In such case, I would use usort().

I can add exception if there is typo dependency written in the function. About sorting seeds in custom logic, I can not imagine a case where it would be used. Why you want to order seeds in custom logic? And why write additional code if there is simple and suitable solution with dependencies...

Also in php 5.3 you can write your class name like that "YourSeedClass::class" to avoid typos.

Sorry for my English. I didn't say I wanted to order seeds in custom logic. I meant that your implementation looks like that it sorts seeds by using two methods - orderSeedsByDependencies() and getSeedDependenciesInstances(). But if you have a method to compare seeds, you can do usort($seeds, [$this, 'compareSeeds']).

Also, people cannot use ::class syntax to avoid typos. Try NonExisitentClass::class. It would also returns NonExisitentClass.

It could be implemented in many ways :). Please write me explicitly what should I fix, to merge this feature (I need it in production urgently...).

OK. Then please don't add additional interface. Other points are minor things. It could be changed later if someone interests.

chinpei215 · 2017-12-10T05:54:36Z

src/Phinx/Seed/OrderedSeedInterface.php

+    /**
+     * @return array
+     */
+    public function getDependencies();


Why do we need an additional interface? I think we can add some additional method to the AbstractSeed class with a default implementation.

Of course we can add method to AbstractSeed. I did interface if some people do not want to order they seeds and then they keep them without this interface and without getDependencies() function.
If there is a need to move to AbstractSeed class I can do it.

stickler-ci · 2017-12-14T06:13:35Z

src/Phinx/Console/Command/Test.php

 The <info>test</info> command verifies the YAML configuration file and optionally an environment

 <info>phinx test</info>
 <info>phinx test -e development</info>

 EOT
-             );
+);


Line indented incorrectly; expected at least 8 spaces, found 0

norkus256 · 2017-12-17T11:20:23Z

@chinpei215

chinpei215

Sorry for repeated requests. As I said above, I want to keep smaller interface. If we add new public/protected methods, we cannot changes those methods without breaking backward compatibility.

chinpei215 · 2017-12-20T05:36:43Z

src/Phinx/Migration/Manager.php

+     *
+     * @return AbstractSeed[]
+     */
+    protected function getSeedDependenciesInstances(AbstractSeed $seed)


Could you please change protected to private.

chinpei215 · 2017-12-20T05:43:08Z

src/Phinx/Migration/Manager.php

+    {
+        if (empty($this->seeds)) {
+            return [];
+        }


This check seems unnecessary. If $this->seeds is an empty array, I think this method is not called.

chinpei215 · 2017-12-20T05:45:54Z

src/Phinx/Migration/Manager.php

+     *
+     * @return AbstractSeed[]
+     */
+    protected function orderSeedsByDependencies(array $seeds)


Could you please change protected to private.

chinpei215 · 2017-12-20T08:22:30Z

Thank you. Looks good to me.

norkus256 · 2017-12-20T08:26:38Z

@chinpei215 if everything is ok, could you merge now those changes?

chinpei215 · 2017-12-20T08:28:38Z

@Emptyhand Merged. Thank you for your contribution.

Shardj · 2018-07-27T16:01:37Z

Hey does this actually work? When I run my seeder with function getDependencies() it still immediately goes into run() and doesn't run the dependency seeders first

Shardj · 2018-07-27T16:33:35Z

I'm going to xdebug through on monday and see if I can find out why it isn't running these. Manager.php 's getSeedDependenciesInstances seems to correctly find them and return them. It does run twice though for some reason, first instance nothing is passed in, presumably it's using a non-extended instance of AbstractSeed then.

norkus256 · 2018-07-27T19:00:14Z

Yes it works for me well. You have to run just console command php vendor/bin/phinx seed:run
without adding any seeds, then dependencies will be executed automatically

Shardj · 2018-07-27T22:08:53Z

Definitely not my experience so far, running the seeder with the dependencies specifically jumps straight to run, and running all seeders just executes them in alphanumeric order and when it reaches the file in question it ignores the dependencies. I'll try to find out why when I'm back at work on Monday, I'll make a pull request if it's a flaw and not something idiotic I did (I suspect namespaces and bad backslash encoding).

Shardj · 2018-07-30T10:31:36Z

No idea why but when I came in today it was working fine, I probably did something stupid, or it's standard PHP wizardry. Sorry for the trouble

stickler-ci reviewed Nov 26, 2017

View reviewed changes

norkus256 force-pushed the feature-ordered-seeds branch from 43413cb to bc9fe48 Compare November 26, 2017 11:25

This was referenced Nov 26, 2017

How to execute seeders in a specific order #1240

Closed

Ordering for database seeding #1096

Closed

Priority run seed #718

Closed

chinpei215 added enhancement waiting on feedback labels Dec 5, 2017

chinpei215 reviewed Dec 10, 2017

View reviewed changes

norkus256 force-pushed the feature-ordered-seeds branch 3 times, most recently from 1b0f373 to d338f31 Compare December 14, 2017 06:04

stickler-ci reviewed Dec 14, 2017

View reviewed changes

norkus256 force-pushed the feature-ordered-seeds branch 6 times, most recently from 357c942 to 6543ca5 Compare December 17, 2017 11:10

chinpei215 reviewed Dec 20, 2017

View reviewed changes

Šarūnas Norkus added 2 commits December 20, 2017 10:13

Add seeds ordering by dependencies

61b28bf

Fix codesniffer version to avoid breaking changes

00ffb18

norkus256 force-pushed the feature-ordered-seeds branch from 6543ca5 to 00ffb18 Compare December 20, 2017 08:14

chinpei215 approved these changes Dec 20, 2017

View reviewed changes

chinpei215 merged commit f0864db into cakephp:master Dec 20, 2017

ccatterina mentioned this pull request Nov 23, 2020

Feature: Seed order cakephp/migrations#405

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ordered seeds feature #1244

Add ordered seeds feature #1244

norkus256 commented Nov 26, 2017 •

edited

Loading

stickler-ci Nov 26, 2017

stickler-ci Nov 26, 2017

stickler-ci Nov 26, 2017

dereuromark Nov 26, 2017

stickler-ci Nov 26, 2017

codecov-io commented Nov 26, 2017 •

edited

Loading

chinpei215 Dec 10, 2017

norkus256 Dec 10, 2017 •

edited

Loading

norkus256 Dec 10, 2017 •

edited

Loading

chinpei215 Dec 11, 2017

norkus256 Dec 11, 2017

chinpei215 Dec 12, 2017

chinpei215 Dec 10, 2017

norkus256 Dec 10, 2017

stickler-ci Dec 14, 2017

norkus256 commented Dec 17, 2017

chinpei215 left a comment

chinpei215 Dec 20, 2017

chinpei215 Dec 20, 2017

chinpei215 Dec 20, 2017 •

edited

Loading

chinpei215 commented Dec 20, 2017

norkus256 commented Dec 20, 2017

chinpei215 commented Dec 20, 2017

Shardj commented Jul 27, 2018 •

edited

Loading

Shardj commented Jul 27, 2018 •

edited

Loading

norkus256 commented Jul 27, 2018

Shardj commented Jul 27, 2018

Shardj commented Jul 30, 2018 •

edited

Loading

Add ordered seeds feature #1244

Add ordered seeds feature #1244

Conversation

norkus256 commented Nov 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Nov 26, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

norkus256 Dec 10, 2017 • edited Loading

Choose a reason for hiding this comment

norkus256 Dec 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norkus256 commented Dec 17, 2017

chinpei215 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinpei215 Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

chinpei215 commented Dec 20, 2017

norkus256 commented Dec 20, 2017

chinpei215 commented Dec 20, 2017

Shardj commented Jul 27, 2018 • edited Loading

Shardj commented Jul 27, 2018 • edited Loading

norkus256 commented Jul 27, 2018

Shardj commented Jul 27, 2018

Shardj commented Jul 30, 2018 • edited Loading

norkus256 commented Nov 26, 2017 •

edited

Loading

codecov-io commented Nov 26, 2017 •

edited

Loading

norkus256 Dec 10, 2017 •

edited

Loading

norkus256 Dec 10, 2017 •

edited

Loading

chinpei215 Dec 20, 2017 •

edited

Loading

Shardj commented Jul 27, 2018 •

edited

Loading

Shardj commented Jul 27, 2018 •

edited

Loading

Shardj commented Jul 30, 2018 •

edited

Loading