feat: Notifications CRD #550
Conversation
| "github.com/google/uuid" | ||
| ) | ||
|
|
||
| func PersistNotificationFromCRD(obj *v1.Notification) error { |
There was a problem hiding this comment.
@moshloop For all the CRD implemented resources, we have a spec column in the corresponding database table. However, notification is already built in a different way. We don't have a spec column for it.
Instead of modifying the notification table by removing existing columns and adding the spec column, I've implemented it this way. So far I don't really see why this would be an issue.
db/notifications.go
Outdated
| if uid, err := uuid.Parse(obj.Spec.To.Person); err == nil { | ||
| dbObj.PersonID = &uid | ||
| } else { | ||
| var person models.Person | ||
| if err := Gorm.Where("email = ?", obj.Spec.To.Person).First(&person).Error; err != nil { | ||
| return err | ||
| } | ||
| dbObj.PersonID = &person.ID | ||
| } |
There was a problem hiding this comment.
Can we move these into a FindPerson/Team? - I also think the person lookup should be uuid, name or email and our docs to use Name so as not confuse with to.email which doesn't require a person object
There was a problem hiding this comment.
I also think the person lookup should be uuid, name or email
people.name is not unique. Should we add that constraint?
adityathebe
force-pushed
the
feat/notification-crd
branch
from
f4fa6c2
to
248df44
Compare
Resolves: #547