Add maintenance notifications in PAC portal during any related mainte…#73
Add maintenance notifications in PAC portal during any related mainte…#73vikasbolla wants to merge 1 commit intoIBM:mainfrom
Conversation
vikasbolla
force-pushed
the
add_maintenance_notification
branch
4 times, most recently
from
d044d95 to
e096e8e
Compare
There was a problem hiding this comment.
I'm thinking of this approach than logging to cluster and updating env causing downtime.
- Maybe maintain the
maintainence statein backend mongoDB with (start date, end date, isMaintainence). - Expose a POST API (strictly restricted to admin), so that the admin can just make API call to update or give them a small toggle button in Admin personna (Enable maintaince or not).
- In UI this would greatly simplify as you just need to make the GET call and if yes provide them the maintainence banner.
vikasbolla
force-pushed
the
add_maintenance_notification
branch
4 times, most recently
from
e32999f to
339ba3f
Compare
| authorized.POST("/feedbacks", services.CreateFeedback) | ||
|
|
||
| // maintenance notification related endpoints | ||
| authorized.GET("/maintenance", services.GetMaintenanceWindows) |
There was a problem hiding this comment.
Shouldnt GetAllMaintenances be only admin route?
There was a problem hiding this comment.
No, We need this call to get the maintenance windows and show the active one to the users
| // MaintenanceListResponse is the public response for listing all maintenance windows | ||
| type MaintenanceListResponse struct { | ||
| Maintenances []MaintenanceResponse `json:"maintenances"` | ||
| } |
There was a problem hiding this comment.
List of all maintenances is not required for public, this would be required for admin only for auditing or some other purposes
| logger.Error("failed to create event", zap.Error(err)) | ||
| } | ||
| }() | ||
| eventLog := fmt.Sprintf("Maintenance window created by admin %s (ID: %s, Enabled: %v)", |
There was a problem hiding this comment.
Can we have the message too here?
There was a problem hiding this comment.
you mean log message?
| if err := dbCon.DeleteMaintenanceWindow(id); err != nil { | ||
| logger.Error("failed to delete maintenance window from db", zap.Error(err)) | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to delete maintenance window"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
For the delete, I feel like doing soft delete with "deleted_at" might be useful, this would help maybe for auditing to know how many maintainces happened for last 6 months etc
There was a problem hiding this comment.
ha true, makes sense, will update this, thanks
| // Validate time range | ||
| if request.EndTime.Before(request.StartTime) || request.EndTime.Equal(request.StartTime) { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "end_time must be after start_time"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
I feel at a given time only one maintainence window can be active. So if a maintainence is already active, then do not allow one more maintainence to be created
There was a problem hiding this comment.
But,I feel we need to allow them creating them new maintenance if they are at difference timestamp but not allow to multiple maintenance notifications to display to users if the time overlaps.
| var request models.MaintenanceUpdateRequest | ||
| if err := utils.BindAndValidate(c, &request); err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request body"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
Similarly in update, you can enable a maintaince window if there are no other maintaince window active
There was a problem hiding this comment.
Same here, we can restrict enabling or creating a maintenance window if the time overlaps
| func DeleteMaintenanceWindow(c *gin.Context) { | ||
| logger := log.GetLogger() | ||
| id := c.Param("id") |
There was a problem hiding this comment.
I'm wondering do we even need DeleteMaintenance endpoint, as an admin we give them the option to disable the maintainence window and do not require to delete it.
So I feel we just need to add a TTL, lets say if the maintainence is disabled for more than 1year, then simply delete from DB.
| // Validate time range if both times are provided | ||
| if !request.StartTime.IsZero() && !request.EndTime.IsZero() { | ||
| if request.EndTime.Before(request.StartTime) || request.EndTime.Equal(request.StartTime) { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "end_time must be after start_time"}) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we restrict the end time to maybe lets say 15 days since the start time since it was created. Otherwise admin might simply keep updating the same maintainence object over and over which will lose the track of how many maintainence windows were created?
vikasbolla
force-pushed
the
add_maintenance_notification
branch
9 times, most recently
from
6ff5563 to
bdfc702
Compare
…nance window Signed-off-by: Vikas <vikas.satyanarayana.bolla@ibm.com>
vikasbolla
force-pushed
the
add_maintenance_notification
branch
from
bdfc702 to
59adc2f
Compare
| // Check if admin wants all windows | ||
| showAll := c.Query("all") == "true" |
There was a problem hiding this comment.
Can we make this admin only check that is only admin can use this "all" query?
There was a problem hiding this comment.
Query param is passed from frontend only for admin
2 participants
Added functionality to display maintenance notifications to PAC users. There is seperate page for admin to manage create, edit, delete, enable/disable maintenance windows.