Skip to content

Instantly share code, notes, and snippets.

@ryanmr
Last active January 30, 2016 13:43
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save ryanmr/065b3f471553b00638ec to your computer and use it in GitHub Desktop.
Save ryanmr/065b3f471553b00638ec to your computer and use it in GitHub Desktop.

A short story about a six month bug

I've been tinkering with Laravel for at least six months ago with a realistic goal of finishing a custom Nexus CMS before I expire of old age. A few months ago, I came around to adding the functionality for Episode events, specifically, applying default hosts and album art to an episode of none were specified when it was initally created -- short of a shortcut feature.

I will be addressing some classes (an Event, and a Listener, and below, a tiny fraction of the Episode model) from this gist published earlier. There might be some minor changes between this and production code, but it suffices.

    public function store(Request $request)
    {

      $input = $request->input('episode');
      $relationships = ['art', 'series', 'people', 'related', 'media'];
      $core = array_except($input, $relationships);


      $episode = Episode::create($core);

      $episode->save();

      $episode = $this->saveRelated($episode, $request);

      // fetch a clean object and return it

      $e = Episode::with($relationships)->find($episode->id);

      return \Response::json(
        [
          'data' => [
            'episode' => $e->toArray()
          ],
          'method' => 'create'
        ]
      );

    }


    private function saveRelated(Episode $episode, Request $request) {
      $episode->people()->sync($request->input('episode.people', []));

      $episode->related()->sync($request->input('episode.related', []));

      $this->saveMedia($episode, $request->input('episode.media', []));

      return $episode;
    }

Basic run down:

  1. Get all of the input
  2. Filter the input so that the basic fields on the model can be saved
  3. Create and then save the core fields of the Episode model
  4. Save the related fields of the model
  5. Save the people relation
  6. Save the related episodes relation
  7. Save the album art related (which apparently requires some tinkering because Vuejs... another story)
  8. Fetch a fresh copy of the Episode model from the database with all relationships eager loaded
  9. Hooray!

Somewhere between 3 and 4, the Episode::created is fired, and the EpisodeWasCreated event is called, which applies ApplyEpisodeDefaults. I am not quite sure why Episode::created does not call ApplyEpisodeDefaults directly, but Laravel knows best, right?

In ApplyEpisodeDefaults, the crux is that it checks if the Episode series has any previously set default hosts, and if so, it attempts to add them automatically to that episode. For example, if I save a new episode of ATN, instead of having to manually add Matt and I, the episode would get Matt and I automatically added once the Episode was created. Shortcut!

See this section for details.

$saved = $episode->people()->sync($data);

I once thought that my Logging statements were causing the event or eloquent to fail, but luckily, neither was true! It was me, all along! And you could have spotted the bug already! You have all the clues.


The bug was caused by my misunderstanding of when created was fired, which was clouded by my interpretation of what consitutes as creating an episode.

When Episode::create($core) is run, it writes that data to the database and fires the created event, which then applies the default hosts if that episode's series has defaults sets. Great. To be honest, the next line, $episode->save() might be redundant because create seems to write directly. Works, either way.

But not more than three lines later, I try running:

$episode->people()->sync($request->input('episode.people', []));

See the problem yet?

You're right! I completely overwrote the ApplyEpisodeDefaults sync function's progress with a blank array sync. I just never thought of that sync as separate from the create so it hadn't occured to me.


I wanted the event to be fired on created, and for it not be overriden by a blank array on created. My solution was to ignore detaching when the model was new.

$episode->people()->sync($request->input('episode.people', []), !$episode->wasRecentlyCreated);

The wasRecentlyCreated property is great -- it is true when the model was created in this request lifecycle, so it's false on subsequent updates when we need [] to detach if that's what our intent is, but otherwise, it works like we want for created.


Lessons learned:

  1. Saving eloquent data is not necessarily atomic; it saves incrementally in steps, unless, perhaps, wrapped up in a transaction, but even then, it's not the same.
  2. Think about what I'm doing before I believe a core package is to blame. Mostly always my fault.
  3. Don't wait six months to fix a bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment