Skip to content

Added support for Laravel Migrations & Reminders #70

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

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

neon64
Copy link

@neon64 neon64 commented Dec 6, 2014

Doctrine automatically updates the database schema to represent the entities in your application, thus rendering most of the use-cases for Laravel's migrations useless, however Laravel's migrations can still be useful for migrating other stuff, such as data stored on the filesystem.

What's Changed

This pull request adds support for Laravel migrations, using Doctrine for the DB interaction. It adds a new Entity: Migration, that is automatically included into the metadata config. It also adds a new implementation of MigrationRepositoryInterface that uses Doctrine instead of Laravel's DB.

I've also added support for Laravel's password reminders.

Caveats:

  • Switching connections doesn't work (perhaps you might know how to fix it?)
  • The Migration entity cannot be removed (perhaps I should add a config option)

It'd be great to know what you think about this, and if it is a good fit for your package.

@kirkbushell
Copy link
Collaborator

This is excellent. Will test it out when I get home.

@neon64
Copy link
Author

neon64 commented Dec 6, 2014

I found a bug with the getRan() method. It should work better now.

@neon64
Copy link
Author

neon64 commented Dec 11, 2014

I've added support for password reminders as well. Let me know if you'd prefer that in a seperate PR.

@neon64 neon64 changed the title Added support for Laravel Migrations Added support for Laravel Migrations & Reminders Dec 11, 2014
Added `@dev` within version constraint so that the rest of a projects dependencies can be stable.
@jonesio
Copy link

jonesio commented Jan 16, 2015

👍

@kirkbushell
Copy link
Collaborator

There's some comments in some of the code that can be removed - and please write tests for the new features.

@kirkbushell
Copy link
Collaborator

@neon64 ping!

@neon64
Copy link
Author

neon64 commented Jan 29, 2015

I've been without stable internet for a couple of days but I'll try to get those fixes done asap.

@neon64
Copy link
Author

neon64 commented Jan 29, 2015

What comments did you want removed? I could only find the // not implemented one in DoctrineMigrationRepository plus a bunch of docblocks, which I assume you want kept.

@neon64
Copy link
Author

neon64 commented Jan 29, 2015

I tried running the existing tests and they failed. Fatal error: Call to undefined method Mitch\LaravelDoctrine\Configuration\SqliteMapper::isAppropriate() in /Users/chris/Web/Personal/Oxygen/workbench/mitchellvanw/laravel-doctrine/tests/Configuration/SqliteMapperTest.php on line 19 I changed the calls to `isAppropriate` to `isAppropriateFor` and now the following test fails Tests\Configuration\SqliteMapperTest::testMapping Failed asserting that two arrays are equal. --- Expected +++ Actual @@ @@ Array ( 'driver' => 'pdo_sqlite' - 'path' => 'path/database/db.sqlite' + 'path' => 'db' 'user' => 'somedude' + 'password' => null )

Sorry. I forgot to merge new changes from upstream. The tests are fine now

@FoxxMD
Copy link
Contributor

FoxxMD commented Apr 28, 2015

👍

…PresenceVerifier`

Needed if the presence verifier is overriden again by the user. Used to go like this:
- validator factory is resolved when $app['validator'] is accessed (in order to call $validator->resolver($foo) )
- validation factory accesses the `validation.presence` verifier
- the presence verifier is accessed, thus the entity manager is requested
- this prompts a connection to the database on every instantiation of the application

Now the connection won't be resolved until the verifier is actually used.
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.

5 participants