GrooveOtter / grooveotter-client

Front-end client for GrooveOtter
http://grooveotter-demo.herokuapp.com/
0 stars 0 forks source link

#245 fix for Newsfeed "complete first task for today" #257

Open Arilas opened 9 years ago

Arilas commented 9 years ago

Depends on https://github.com/GrooveOtter/grooveotter-api/pull/24 because previous notifications created without task.

Arilas commented 9 years ago

Also, previous Notifications about 'first_task' we cannot like, because it's doesn't related to any task

garrethdev commented 9 years ago

Bug still happening on branch. I updated the server side too, and still no luck. It seems to be the same issue.

screen shot 2015-09-11 at 10 57 23 am
Arilas commented 9 years ago

As I said, "like" will work only for new Notifications, old ones doesn't have task, so like will not work for it. We can remove old notifications with type 'notification'

garrethdev commented 9 years ago

Yea this is a new notification. I accessed the heroku postgres instance and deleted all Notifications before I tested this and created a new one.

Arilas commented 9 years ago

Is this instance contains changes from this: https://github.com/GrooveOtter/grooveotter-api/pull/24 ? Because only after this change, it's should work. Can you see table notifications in DB, task_id should be populated in all notifications?

garrethdev commented 9 years ago

@Arilas yea that api branch is already merged into master. You should be able to test this by deleting Notifications, and removing the Cookie TaskDay on the Client side. To access the DB:

  1. Run heroku pg:psql --app whispering-scrubland-7949
  2. Run DELETE FROM notifications WHERE type = 'notification';

From there you would just need to complete a few tasks to create a notification or just Post to the API as a notification

Arilas commented 9 years ago

I've checked again and it's works on local machine. And also I see that from server side going list with empty task_id column. Can you please update instance? Because as I see, pull request to API side is merged after creation of this instance

garrethdev commented 9 years ago

@Arilas Im not quite clear what besides this needs to be merged into master? https://github.com/GrooveOtter/grooveotter-api/pull/24

Arilas commented 9 years ago

As I see, Client send 'task_id' to server, but server doesn't save it(You can look at db, new notifications doesn't have this field). This bug was fixed in https://github.com/GrooveOtter/grooveotter-api/pull/24 that was successfully merged. But as I see, it's was merged after this heroku inastance was created, so it's doesn't have this changes.

garrethdev commented 9 years ago

@Arilas If you do a heroku restart or clear cache that should fix it. Or if you redeploy.

garrethdev commented 9 years ago
screen shot 2015-09-15 at 7 58 16 am

After reloading the page

screen shot 2015-09-15 at 8 01 18 am
Arilas commented 9 years ago

@GarrethDottin Is this instance uses the same database as live? Because If yes, changes doesn't work because notifications is populated from live site, that doesn't contains task_id field, because it's client side doesn't send it to server.

garrethdev commented 9 years ago

@Arilas The client side that was testing this was done through the PR request. So any information was sent through a client side that should have the latest changes. If we're still having issues, we should probably review with Tenor @L8D as we talked about before.

Also when I was testing this morning, it was a case of me using the PR environment.

Arilas commented 9 years ago

@Ryan-Parker @GarrethDottin Fixed. Notifications(created in this instance) now Like successfully

garrethdev commented 9 years ago

@Arilas Still Seeing this I created it under the pr request.

screen shot 2015-09-15 at 2 25 10 pm
garrethdev commented 9 years ago

If you want to test this on the environment I would recommend going into resources in the console and deleting the taskDay in localStorage. From there just start the timer and hit complete timer a bunch of times. This will eventually create a notification

Arilas commented 9 years ago

@Ryan-Parker fixed this. Please run task for at least 5 seconds before click on Complete.