FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: address PR review comments for send schedule feature · jvcss/httpsms@2b9a45f · GitHub

/ httpsms Public
forked from NdoleStudio/httpsms

Commit 2b9a45f

Browse files
AchoArnold
and
Copilot
committed
fix: address PR review comments for send schedule feature
- Add schedule ownership check in phone handler to prevent cross-user schedule_id assignment (security fix) - Fix OpenAPI annotation for Index endpoint to use wrapper response type - Fix 402 response annotation to use proper PaymentRequired type - Add PaymentRequired response type to responses package - Mark schedule_id as optional in PhoneUpsert request struct - Fix scheduleWindowError matching to use backend error format (day_of_week) - Remove unnecessary Promise.all wrapping in store actions - Remove unused limit query param from getSendSchedules - Treat active schedule with empty windows as inactive (send immediately) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 93d8ac9 commit 2b9a45f

8 files changed

Lines changed: 68 additions & 56 deletions

File tree

‎api/pkg/di/container.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,7 @@ func (container *Container) PhoneHandler() (handler *handlers.PhoneHandler) {
11171117
container.Logger(),
11181118
container.Tracer(),
11191119
container.PhoneService(),
1120+
container.MessageSendScheduleService(),
11201121
container.PhoneHandlerValidator(),
11211122
)
11221123
}

‎api/pkg/entities/message_send_schedule.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,14 @@ type MessageSendSchedule struct {
2727

2828
// ResolveScheduledAt returns the next allowed send time based on the schedule.
2929
// If the schedule is inactive, has no windows, or has an invalid timezone,
30-
// the current time is returned in UTC.
30+
// the current time is returned in UTC. An active schedule with no windows
31+
// is treated as inactive (messages are sent immediately).
3132
func (schedule *MessageSendSchedule) ResolveScheduledAt(current time.Time) time.Time {
32-
if schedule == nil || !schedule.IsActive || len(schedule.Windows) == 0 {
33+
if schedule == nil || !schedule.IsActive {
34+
return current.UTC()
35+
}
36+
37+
if len(schedule.Windows) == 0 {
3338
return current.UTC()
3439
}
3540

‎api/pkg/handlers/message_send_schedule_handler.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (h *MessageSendScheduleHandler) RegisterRoutes(router fiber.Router, middlew
5656
// @Security ApiKeyAuth
5757
// @Tags Send Schedules
5858
// @Produce json
59-
// @Success 200 {array} entities.MessageSendSchedule
59+
// @Success 200 {object} responses.MessageSendSchedulesResponse
6060
// @Failure 401 {object} responses.Unauthorized
6161
// @Failure 500 {object} responses.InternalServerError
6262
// @Router /send-schedules [get]
@@ -85,7 +85,7 @@ func (h *MessageSendScheduleHandler) Index(c *fiber.Ctx) error {
8585
// @Success 201 {object} responses.MessageSendScheduleResponse
8686
// @Failure 400 {object} responses.BadRequest
8787
// @Failure 401 {object} responses.Unauthorized
88-
// @Failure 402 {object} responses.BadRequest
88+
// @Failure 402 {object} responses.PaymentRequired
8989
// @Failure 422 {object} responses.UnprocessableEntity
9090
// @Failure 500 {object} responses.InternalServerError
9191
// @Router /send-schedules [post]

‎api/pkg/handlers/phone_handler.go‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package handlers
22

33
import (
44
"fmt"
5+
"net/url"
6+
"strings"
57

68
"github.com/NdoleStudio/httpsms/pkg/requests"
79
"github.com/NdoleStudio/httpsms/pkg/validators"
810
"github.com/davecgh/go-spew/spew"
11+
"github.com/google/uuid"
912

1013
"github.com/NdoleStudio/httpsms/pkg/services"
1114
"github.com/NdoleStudio/httpsms/pkg/telemetry"
@@ -16,24 +19,27 @@ import (
1619
// PhoneHandler handles phone http requests.
1720
type PhoneHandler struct {
1821
handler
19-
logger telemetry.Logger
20-
tracer telemetry.Tracer
21-
service *services.PhoneService
22-
validator *validators.PhoneHandlerValidator
22+
logger telemetry.Logger
23+
tracer telemetry.Tracer
24+
service *services.PhoneService
25+
scheduleService *services.MessageSendScheduleService
26+
validator *validators.PhoneHandlerValidator
2327
}
2428

2529
// NewPhoneHandler creates a new PhoneHandler
2630
func NewPhoneHandler(
2731
logger telemetry.Logger,
2832
tracer telemetry.Tracer,
2933
service *services.PhoneService,
34+
scheduleService *services.MessageSendScheduleService,
3035
validator *validators.PhoneHandlerValidator,
3136
) (h *PhoneHandler) {
3237
return &PhoneHandler{
33-
logger: logger.WithService(fmt.Sprintf("%T", h)),
34-
tracer: tracer,
35-
validator: validator,
36-
service: service,
38+
logger: logger.WithService(fmt.Sprintf("%T", h)),
39+
tracer: tracer,
40+
validator: validator,
41+
service: service,
42+
scheduleService: scheduleService,
3743
}
3844
}
3945

@@ -127,6 +133,15 @@ func (h *PhoneHandler) Upsert(c *fiber.Ctx) error {
127133
return h.responseUnprocessableEntity(c, errors, "validation errors while updating phones")
128134
}
129135

136+
if request.ScheduleID != nil && strings.TrimSpace(*request.ScheduleID) != "" {
137+
scheduleID, _ := uuid.Parse(strings.TrimSpace(*request.ScheduleID))
138+
if _, err := h.scheduleService.Load(ctx, h.userFromContext(c).ID, scheduleID); err != nil {
139+
validationErrors := url.Values{}
140+
validationErrors.Add("schedule_id", "schedule_id does not belong to the authenticated user or does not exist")
141+
return h.responseUnprocessableEntity(c, validationErrors, "validation errors while updating phones")
142+
}
143+
}
144+
130145
phone, err := h.service.Upsert(ctx, request.ToUpsertParams(h.userFromContext(c), c.OriginalURL()))
131146
if err != nil {
132147
msg := fmt.Sprintf("cannot update phones with params [%+#v]", request)

‎api/pkg/requests/phone_update_request.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type PhoneUpsert struct {
3131
// SIM is the SIM slot of the phone in case the phone has more than 1 SIM slot
3232
SIM string `json:"sim" example:"SIM1"`
3333

34-
ScheduleID *string `json:"schedule_id" example:"32343a19-da5e-4b1b-a767-3298a73703cb"`
34+
ScheduleID *string `json:"schedule_id,omitempty" example:"32343a19-da5e-4b1b-a767-3298a73703cb" validate:"optional"`
3535
}
3636

3737
// Sanitize sets defaults to MessageOutstanding

‎api/pkg/responses/response.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ type Unauthorized struct {
3838
Data string `json:"data" example:"Make sure your API key is set in the [X-API-Key] header in the request"`
3939
}
4040

41+
// PaymentRequired is the response with status code is 402
42+
type PaymentRequired struct {
43+
Status string `json:"status" example:"error"`
44+
Message string `json:"message" example:"You have reached the maximum number of allowed resources. Please upgrade your plan."`
45+
}
46+
4147
// NoContent is the response when status code is 204
4248
type NoContent struct {
4349
Status string `json:"status" example:"success"`

‎web/pages/settings/index.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,10 +1779,10 @@ export default Vue.extend({
17791779
}
17801780
17811781
const message = messages.find((x: string) =>
1782-
x.includes(`Day of week ${index}`),
1782+
x.includes(`day_of_week ${index}`),
17831783
)
17841784
return message
1785-
? message.replace(`Day of week ${index}`, this.getWeekday(index))
1785+
? message.replace(`day_of_week ${index}`, this.getWeekday(index))
17861786
: null
17871787
},
17881788

‎web/store/index.ts‎

Lines changed: 26 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,23 +1111,17 @@ export const actions = {
11111111
getSendSchedules(context: ActionContext<State, State>) {
11121112
return new Promise<Array<EntitiesSendSchedule>>((resolve, reject) => {
11131113
axios
1114-
.get<ResponsesSendSchedulesResponse>(`/v1/send-schedules`, {
1115-
params: {
1116-
limit: 100,
1117-
},
1118-
})
1114+
.get<ResponsesSendSchedulesResponse>(`/v1/send-schedules`)
11191115
.then((response: AxiosResponse<ResponsesSendSchedulesResponse>) => {
11201116
resolve(response.data.data)
11211117
})
11221118
.catch(async (error: AxiosError) => {
1123-
await Promise.all([
1124-
context.dispatch('addNotification', {
1125-
message:
1126-
(error.response?.data as any)?.message ??
1127-
'Error while fetching send schedules',
1128-
type: 'error',
1129-
}),
1130-
])
1119+
await context.dispatch('addNotification', {
1120+
message:
1121+
(error.response?.data as any)?.message ??
1122+
'Error while fetching send schedules',
1123+
type: 'error',
1124+
})
11311125
reject(getErrorMessages(error))
11321126
})
11331127
})
@@ -1144,14 +1138,12 @@ export const actions = {
11441138
resolve(response.data.data)
11451139
})
11461140
.catch(async (error: AxiosError) => {
1147-
await Promise.all([
1148-
context.dispatch('addNotification', {
1149-
message:
1150-
(error.response?.data as any)?.message ??
1151-
'Error while creating send schedule',
1152-
type: 'error',
1153-
}),
1154-
])
1141+
await context.dispatch('addNotification', {
1142+
message:
1143+
(error.response?.data as any)?.message ??
1144+
'Error while creating send schedule',
1145+
type: 'error',
1146+
})
11551147
reject(getErrorMessages(error))
11561148
})
11571149
})
@@ -1171,38 +1163,31 @@ export const actions = {
11711163
resolve(response.data.data)
11721164
})
11731165
.catch(async (error: AxiosError) => {
1174-
await Promise.all([
1175-
context.dispatch('addNotification', {
1176-
message:
1177-
(error.response?.data as any)?.message ??
1178-
'Error while updating send schedule',
1179-
type: 'error',
1180-
}),
1181-
])
1166+
await context.dispatch('addNotification', {
1167+
message:
1168+
(error.response?.data as any)?.message ??
1169+
'Error while updating send schedule',
1170+
type: 'error',
1171+
})
11821172
reject(getErrorMessages(error))
11831173
})
11841174
})
11851175
},
11861176

1187-
deleteSendSchedule(
1188-
context: ActionContext<State, State>,
1189-
payload: string,
1190-
) {
1177+
deleteSendSchedule(context: ActionContext<State, State>, payload: string) {
11911178
return new Promise<void>((resolve, reject) => {
11921179
axios
11931180
.delete<ResponsesNoContent>(`/v1/send-schedules/${payload}`)
11941181
.then(() => {
11951182
resolve()
11961183
})
11971184
.catch(async (error: AxiosError) => {
1198-
await Promise.all([
1199-
context.dispatch('addNotification', {
1200-
message:
1201-
(error.response?.data as any)?.message ??
1202-
'Error while deleting send schedule',
1203-
type: 'error',
1204-
}),
1205-
])
1185+
await context.dispatch('addNotification', {
1186+
message:
1187+
(error.response?.data as any)?.message ??
1188+
'Error while deleting send schedule',
1189+
type: 'error',
1190+
})
12061191
reject(getErrorMessages(error))
12071192
})
12081193
})

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL