JeffreyWay / council

An open source forum built on Laravel.
MIT License
619 stars 207 forks source link

User notifications throw 'undefined' error #224

Closed Douglasdc3 closed 6 years ago

Douglasdc3 commented 6 years ago

When you receive a notification the notification pop up doesn't load. this is related to the vue component trying to fetch the notifier property which does not exist inside the data property.

The /profiles/John%20Doe/notifications response is

[
  {
    "id": "6e750b00-7d21-4154-8480-83cd6c231ab5",
    "type": "App\\Notifications\\ThreadWasUpdated",
    "notifiable_type": "App\\User",
    "notifiable_id": 1,
    "data": {
      "message": "Marty McFly replied to Quidem esse harum itaque occaecati.",
      "link": "\/threads\/laravel-mix\/quidem-esse-harum-itaque-occaecati-33?page=1#reply-11"
    },
    "read_at": null,
    "created_at": "2018-03-16 06:57:22",
    "updated_at": "2018-03-16 06:57:22"
  }
]

giving the following error

app.js:70178 [Vue warn]: Error in render: "TypeError: Cannot read property 'avatar_path' of undefined"

found in

---> <UserNotifications> at resources/assets/js/components/UserNotifications.vue

app.js:71315 TypeError: Cannot read property 'avatar_path' of undefined
    at app.js:82125
    at Proxy.renderList (app.js:73270)
    at Proxy.render (app.js:82100)
    at VueComponent.Vue._render (app.js:74072)
    at VueComponent.updateComponent (app.js:72363)
    at Watcher.get (app.js:72713)
    at Watcher.run (app.js:72790)
    at flushSchedulerQueue (app.js:72552)
    at Array.<anonymous> (app.js:71411)
    at flushCallbacks (app.js:71332)

The UserNotification.vue component references this property in it's template

  <a :href="notification.data.link"
        class="text-xs flex items-center pr-1 link"
        @click.prevent="markAsRead(notification)"
  >
    <img :src="notification.data.notifier.avatar_path"
              :alt="notification.data.notifier.username"
              class="w-8 mr-3">

    <span v-text="notification.data.message"></span>
  </a>
JeffreyWay commented 6 years ago

Have you recompiled your JavaScript recently? I just checked this, and the notifier property is there.

Douglasdc3 commented 6 years ago

Tried it again pulling in latest code. Did a artisan migrate:refresh --seed did a yarn install && yarn dev

When I hit the api endpoint I still don't see the notifier property.

I digged into the code found that the ThreadWasUpdated.php toArray method does not return a notifier property

When adding I replaced the method with the snippet below it did work.

    /**
     * Get the array representation of the notification.
     *
     * @return array
     */
    public function toArray()
    {
        return [
            'message' => $this->reply->owner->name.' replied to '.$this->thread->title,
            'notifier' => $this->reply->owner,
            'link' => $this->reply->path()
        ];
    }

I'll submit a PR shortly if you see another reason why this might not be working feel free to just close it.

UPDATE: @JeffreyWay The YouWereMentioned does contain this property the ThreadWasUpdated does not.