Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

feat(notifications_service): adds endpoint to list registered devices of a user #339

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion gateway/services/notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

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"

"GET",
"/notifications/subscriptions",
Copy link
Contributor

Choose a reason for hiding this comment

The 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",
Copy link
Contributor

Choose a reason for hiding this comment

The 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",
Copy link
Contributor

Choose a reason for hiding this comment

The 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",
Expand Down Expand Up @@ -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)
}
Expand Down
43 changes: 41 additions & 2 deletions services/notifications/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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."))
Copy link
Contributor

Choose a reason for hiding this comment

The 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."))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change this too

Copy link
Author

Choose a reason for hiding this comment

The 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
*/
Expand Down