Event point system

[Nydauron]

• Event model now have 3 fields for points: InPersonPoints, InPersonVirtPoints, and VirtualPoints
• Event code model has an additional field IsVirtual to distinguish between code for virtual/in-person use
• Changed PUT /event/code/{id} to POST /event/code/
• Added an admin-only internal route GET /profile/user/{id}/ so that we can determine if the user redeeming the event is virtual or in-person
• Fixed a bug where new entries into profileattendance were resulting in duplicate rows with empty id fields
• Reimplemented POST /event/checkin to check the user for virtual status
• Updated tests for profile and event service
• Updated docs for profile and event

[lauzadis]

Looks very good. I had a few comments.

[lauzadis]

I am wondering why the selector was changed from id to code?

correct me if I'm wrong, but since we are trying to update the code of the event, and you are also searching the database for an event with the new code instead of the old code (or ideally, the unique id), it won't find the correct event.

The old code worked because we grabbed the event ID from PUT /event/code/{id} but since you removed that, I feel we are missing some information that is necessary to do this database update

[Nydauron]

Ok so, I understand the issue at hand, but with the current setup, the table will have multiple fields where id can be equal (since events can have a virtual and an in-person code). The idea behind switching the primary key to code is to fix this unique identifier issue.

What I'm realizing now is that while the current POST route works perfectly for updating information for id, isVirtual, and expiration, we can't change code without deleting the old one and creating a new one. A potential solution could be to make the route POST /event/code/{old_code_id}. That way we can uniquely identify which code to modify and have the ability to modify the code id.

Also, for clarity in the model, I'll probably change the EventCode struct to:

type EventCode struct {
	CodeID     string `json:"codeID"`
	EventID    string `json:"eventID"`
	IsVirtual  bool   `json:"isVirtual"`
	Expiration int64  `json:"expiration"`
}

[lauzadis]

Yeah I think using {old_code} is a great way to fix it. I think we have two options.

1. Change the endpoint to POST /event/code/{old_code_id} so we can update the correct object
2. Don't allow API users to modify the code at all

I think 2 is a feasible option because once the random code is generated once, I don't see many reasons to change it afterwards.

[Nydauron]

Alright then. In both cases, would it be good to also have a DELETE route? Right now, the only way a code gets deleted is if its expiration datetime exceeds the current time and some user tries to utilize said code.

[lauzadis]

Yes that would probably be good to have, but not necessary for this PR in my opinion

[Nydauron]

Alrighty, went ahead and implemented option 2. Also added a little note POST /event/code in the docs saying that codes can't be updated.

[Nydauron]

We should state this explicitly though, in an explicit error message, and a note in the documentation.

As of right now, it's impossible to determine if the user is attempting to change code. Since the method is essentially upserting, if the code simply doesn't exist, it creates a new row.

A possible solution might be to do something like POST /event/code/{code}, but I feel then it would be a bit counterintuitive to not implement option 1.

[lauzadis]

I think it is possible to prevent the user from changing the event struct's code here.

After creating the selector, you can use findOne to get the old event, then set the new eventCode struct's code field to that, and then finally do the upsert.

Here is a rough idea of what I was thinking

selector := database.QuerySelector{
	"codeid": code,
}
// load the old code
var oldEventCode models.EventCode  
err := db.FindOne("eventcodes", selector, &oldEventCode)

// update the code field to make sure it is not changed by the user
eventCode.code = oldEventCode.code

// then do the upsert as normal
_, err := db.Upsert("eventcodes", selector, &eventCode)

You can wrap the findOne in an if-statement. If the eventCode does not already exist, you can run the upsert directly

[tanyongzhi]

selector := database.QuerySelector{
	"codeid": code,
}

Is this assuming that the code variable is the old code? That would be implementing option 1), right?

In my opinion, I think it is alright to create a new row if a new code is specified. Putting the responsibility of calling this endpoint with the correct code can be placed on the caller, instead.

[Nydauron]

I think it is possible to prevent the user from changing the event struct's code here.

After creating the selector, you can use findOne to get the old event, then set the new eventCode struct's code field to that, and then finally do the upsert.

Here is a rough idea of what I was thinking

selector := database.QuerySelector{
	"codeid": code,
}
// load the old code
var oldEventCode models.EventCode  
err := db.FindOne("eventcodes", selector, &oldEventCode)

// update the code field to make sure it is not changed by the user
eventCode.code = oldEventCode.code

// then do the upsert as normal
_, err := db.Upsert("eventcodes", selector, &eventCode)

You can wrap the findOne in an if-statement. If the eventCode does not already exist, you can run the upsert directly

The current implementation works perfectly fine. In all cases when something is being upserted, it will either be updated if code is found or inserted otherwise.

For clarification on what I said, I was building on what Yong commented on; if we were to return some error to the user saying that they are modifying the code, we would probably need something like POST /event/code/{old_code}. It's rather a small nit really since it should be clear that the code can't be changed.

[tanyongzhi]

LGTM, left some nits

[tanyongzhi]

I agree with Matas on not letting users to modify the code id. We should state this explicitly though, in an explicit error message, and a note in the documentation.

[DevSatpathy]

We need to add back the avatar URL, so we can randomly generate a profile picture and affix it to the profile.

[Nydauron]

we can randomly generate a profile picture and affix it to the profile

So, are we setting this to some URL on profile creation?