mybb / mybb2

The repository for the MyBB 2 forum software. Not to be used on live boards.
https://www.mybb.com
BSD 3-Clause "New" or "Revised" License
111 stars 45 forks source link

Use repositories instead models #224

Closed Matslom closed 8 years ago

euantorano commented 8 years ago

Couple of small comments there, the style checks can be ignored for now.

Matslom commented 8 years ago

I wanted also add timestamps on creating user. After disabling them in model, there is no way to use with create method. Can we use something like this UserRepository.php $user = new $this->userModel; $user->name = $details['name']; $user->email = $details['email']; ... $user->timestamps = true; $user->save(); instead of this $this->userModel->create($details)?

euantorano commented 8 years ago

That slight work, or you could set time stamps before create, then reset it.

On 27 Jun 2016, at 21:32, Matslom notifications@github.com wrote:

I wanted also add timestamps on creating user. After disabling them in model, there is no way to use with create method. Can we use something like this UserRepository.php $user = new $this->userModel; $user->name = $details['name']; $user->email = $details['email']; ... $user->timestamps = true; $user->save();

instead of this $this->userModel->create($details)?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

JoshHarmon commented 8 years ago

My thought would be

public function create(array $details)
{
    // manually set timestamps after creating the model
    $user = $this->userModel->create($details);
    $user->touch();
    return $user;
}

I would think that would work, but I have yet to test it. $timestamps should only affect whether or not Eloquent tries to automatically update timestamps. This should manually do so.

Matslom commented 8 years ago

Unfortunately, not really

public function touch()
    {
        if (! $this->timestamps) {
            return false;
        }
JoshHarmon commented 8 years ago

Well... That's unfortunate. Try...

public function create(array $details)
{
    // manually set timestamps after creating the model
    $user = $this->userModel->create($details);
    return $user->setCreatedAt($user->getFreshTimestamp())
                ->setUpdatedAt($user->getFreshTimestamp());
}

That should do things in an acceptable way and return the model (at least according to the API docs)

Matslom commented 8 years ago

Yep, it works

$user->setCreatedAt($user->freshTimestamp())
            ->setUpdatedAt($user->freshTimestamp())->save();
JoshHarmon commented 8 years ago

Also, it looks like you're using tabs. We're not using tabs anymore as of today, so if you could switch to 4 spaces per "tab", that would be great. Otherwise the code sniffer on Travis CI will freak out when we get this PR to go to it. To see if stuff is formatted correctly, you can run composer sniff.

Matslom commented 8 years ago

:smile: I used 4 spaces in first commit. No problem, I will change it.

JoshHarmon commented 8 years ago

Sorry! We passed an internal RFC recently to change the standards to be plain PSR-2 with the prefix/suffix requirements added on!