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

404 when selecting unread notification to read thread #1320

Open
HipsterBrown opened this Issue Dec 2, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@HipsterBrown

HipsterBrown commented Dec 2, 2018

Probably related to #240. When viewing unread notifications and selecting one to read the thread, there is a 404 response in the console and an infinite spinner in the thread panel of the UI.

Are you experiencing this issue in octobox.io or your own instance?

  • octobox.io
  • My instance

🐞 Problem
I suspect the issue is related to the scoping of notifications in the NotificationsController#show as the selected notification was "unread" before clicked but is changed to "read" as it is clicked to open the thread panel. So the controller cannot find the notification from the request (https://octobox.io/notifications/:id?per_page=60&unread=true) because the notification is no longer scoped to "unread".

💡 Possible solutions
For threaded notifications in the app, could there be a way to not scope the requested unread thread in the controller? Because there are notifications that open in a new tab, which you probably want to mark as read immediately, it could be tricky to add logic to the "show" method that find any notification matching that :id for the thread panel and ignore the "unread` param.

📋 Steps to solve the problem

I could take a look at a potential fix this week as I've worked in Rails in the past. Maybe using find_notification instead of scope.find(params[:id]) would be the quickest fix but I'm not sure about the impact that would have to the app yet.

@evexoio

This comment has been minimized.

Contributor

evexoio commented Dec 5, 2018

@HipsterBrown I can't reproduce this, can you help me with more precise reproduction steps?

I tried going to the unread notification page, https://octobox.io/?unread=true
then I opened the console,
and clicked on an unread notification,
a new browser tab opened and I was sent to the github's issue page,
when I got back to the unread notifications page, the console was empty and no errors appeared.

am I missing something?

@HipsterBrown

This comment has been minimized.

HipsterBrown commented Dec 5, 2018

@evexoio This happens for notifications that open threads in octobox.
screen shot 2018-12-05 at 10 15 14 am

@evexoio

This comment has been minimized.

Contributor

evexoio commented Dec 5, 2018

Still couldn't reproduce, can it be repo-specific?

dec-05-2018 17-35-45

@HipsterBrown

This comment has been minimized.

HipsterBrown commented Dec 5, 2018

@evexoio If you view the unread notifications, then click on a notification, that will reproduce the error. In the search box: "inbox:true unread:true"

Thanks for looking into this. 👍

@evexoio

This comment has been minimized.

Contributor

evexoio commented Dec 5, 2018

@HipsterBrown Thanks for clarifying the steps to reproduce the error.
So now I can reproduce the error on octobox.io but not locally.

I can't fix it unless I can reproduce it locally 🤔

@HipsterBrown

This comment has been minimized.

HipsterBrown commented Dec 6, 2018

@evexoio I get that. I'm not sure how to test the in-app threads locally. 🤷‍♂️

@evexoio

This comment has been minimized.

Contributor

evexoio commented Dec 6, 2018

@andrew is there a way to populate the db with in-app threads?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment