-
Notifications
You must be signed in to change notification settings - Fork 51
feat(notifications_service): adds endpoint to list registered devices of a user #339
base: staging
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,14 +5,27 @@ import ( | |
"github.com/HackIllinois/api/gateway/middleware" | ||
"github.com/HackIllinois/api/gateway/models" | ||
|
||
"net/http" | ||
|
||
"github.com/arbor-dev/arbor" | ||
"github.com/justinas/alice" | ||
"net/http" | ||
) | ||
|
||
const NotificationsFormat string = "JSON" | ||
|
||
var NotificationsRoutes = arbor.RouteCollection{ | ||
arbor.Route{ | ||
"GetAllSubscriptions", | ||
"GET", | ||
"/notifications/subscriptions", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a "/" to the end |
||
alice.New(middleware.IdentificationMiddleware, middleware.AuthMiddleware([]models.Role{models.UserRole})).ThenFunc(GetAllSubscriptions).ServeHTTP, | ||
}, | ||
arbor.Route{ | ||
"GetAllRegisteredDevices", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think something more like "GetUserRegisteredDevices" might be better, since you're getting the registered devices of a specific user, not all users. |
||
"GET", | ||
"/notifications/devices", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
alice.New(middleware.IdentificationMiddleware, middleware.AuthMiddleware([]models.Role{models.UserRole})).ThenFunc(GetRegisteredDevices).ServeHTTP, | ||
}, | ||
arbor.Route{ | ||
"GetAllTopics", | ||
"GET", | ||
|
@@ -81,6 +94,14 @@ var NotificationsRoutes = arbor.RouteCollection{ | |
}, | ||
} | ||
|
||
func GetAllSubscriptions(w http.ResponseWriter, r *http.Request) { | ||
arbor.GET(w, config.NOTIFICATIONS_SERVICE+r.URL.String(), NotificationsFormat, "", r) | ||
} | ||
|
||
func GetRegisteredDevices(w http.ResponseWriter, r *http.Request) { | ||
arbor.GET(w, config.NOTIFICATIONS_SERVICE+r.URL.String(), NotificationsFormat, "", r) | ||
} | ||
|
||
func GetAllTopics(w http.ResponseWriter, r *http.Request) { | ||
arbor.GET(w, config.NOTIFICATIONS_SERVICE+r.URL.String(), NotificationsFormat, "", r) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,14 @@ package controller | |
|
||
import ( | ||
"encoding/json" | ||
"net/http" | ||
"time" | ||
|
||
"github.com/HackIllinois/api/common/errors" | ||
"github.com/HackIllinois/api/common/utils" | ||
"github.com/HackIllinois/api/services/notifications/models" | ||
"github.com/HackIllinois/api/services/notifications/service" | ||
"github.com/gorilla/mux" | ||
"net/http" | ||
"time" | ||
) | ||
|
||
func SetupController(route *mux.Route) { | ||
|
@@ -27,6 +28,44 @@ func SetupController(route *mux.Route) { | |
router.HandleFunc("/order/{id}/", GetNotificationOrder).Methods("GET") | ||
} | ||
|
||
/* | ||
Returns all topics a user is subscribed to | ||
*/ | ||
func GetAllSubscriptions(w http.ResponseWriter, r *http.Request) { | ||
id := r.Header.Get("HackIllinois-Identity") | ||
topics, err := service.GetSubscriptions(id) | ||
|
||
if err != nil { | ||
errors.WriteError(w, r, errors.DatabaseError(err.Error(), "Could not retrieve notifications.")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error message should be changed from "notifications" to "subscriptions" |
||
return | ||
} | ||
|
||
topic_list := models.TopicList{ | ||
Topics: topics, | ||
} | ||
|
||
json.NewEncoder(w).Encode(topic_list) | ||
} | ||
|
||
/* | ||
Returns devices registered of a specific user | ||
*/ | ||
func GetRegisteredDevices(w http.ResponseWriter, r *http.Request) { | ||
id := r.Header.Get("HackIllinois-Identity") | ||
devices, err := service.GetUserDevices(id) | ||
|
||
if err != nil { | ||
errors.WriteError(w, r, errors.DatabaseError(err.Error(), "Could not retrieve notifications.")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change this too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Heesoo, After revisiting, I think you're right. My interpretation of the PR was a bit different. Based on the work done so far, I'd like to merge these into master and hopefully open a new PR to address #247 more properly soon. Additionally, I think this can count as my "coding challenge" for the API team right? Please get back to me when you get a chance : ) |
||
return | ||
} | ||
|
||
device_list := models.DeviceList{ | ||
Devices: devices, | ||
} | ||
|
||
json.NewEncoder(w).Encode(device_list) | ||
} | ||
|
||
/* | ||
Returns all topics that notifications can be published to | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing as my comment below about the naming -- "GetUserSubscriptions"