Refactoring - Remove unused endpoints and use Apollo Client 2.0 #45
Conversation
|
Hey y'all! A deployment of this PR can be found here: |
kexin-zhang
changed the title
client/js/main.ts
Outdated
| button.disabled = false; | ||
| }); | ||
| } | ||
|
|
||
| function attachUserDeleteHandlers () { |
There was a problem hiding this comment.
Why is this admin account deletion functionality being removed?
There was a problem hiding this comment.
Honestly, I think I just did this by accident while moving things around. Thanks for pointing this out!
client/js/main.ts
Outdated
| // Escape possible HTML in username | ||
| if (by === null) { |
There was a problem hiding this comment.
Any reason to do this instead of an optional argument?
There was a problem hiding this comment.
I originally did this because the type of checked_in_by in the GraphQL Tag object is string | null. But GraphQL queries should always return a string for checked_in_by (see https://github.com/HackGT/checkin2/blob/master/server/graphql.ts#L129), so I just went ahead and made this a non-null type.
client/js/main.ts
Outdated
|
|
||
| // Get checked question options | ||
| let checked: string[] = []; | ||
| let checked = []; |
There was a problem hiding this comment.
This list will be implicitly typed as any if you remove this annotation
| @@ -645,44 +515,8 @@ document.getElementById("confirmation-branches-filter")!.addEventListener("chang | |||
| loadAttendees(); | |||
| }); | |||
|
|
|||
| attachUserDeleteHandlers(); | |||
There was a problem hiding this comment.
See above comment about user management
server/app.ts
Outdated
| fs.createReadStream(request.file.path).pipe(parser); | ||
| }); | ||
|
|
||
| apiRouter.route("/data/export").get(authenticateWithReject, async (request, response) => { |
There was a problem hiding this comment.
Is CSV data export something worth keeping around? Related to #48
There was a problem hiding this comment.
Yeah, this is a good point. Let's keep CSV export, but I do think we should get around to #48 at some point, since the CSV export exports data in checkin's mongo collection, which is only a subset of all the data that checkin GraphQL can provide.
|
@petschekr thanks for taking a look! Made some changes based on your comments. |
Changes:
testfolder, so that the tests get compiled to JS.