Fix exec fetching and add events #491

Merged
a258wang merged 7 commits from amy-news-2022-09-01 into main 5 months ago
Owner
  • Fixed edge case with fetching execs
  • Added Bootcamp mentor application news, and CSC x Google event

Problem: When building the Meet the Team page using LDAP (in CI), we were only checking CSC members for the "current" term (Fall 2022) to see if any of them were execs. Since our actual current execs (the Spring 2022 execs, since Fall 2022 elections have not occurred yet) have not renewed their memberships for Fall 2022, the script was unable to find individuals for the Prez/VP/AVP/Trez positions, which caused an error.

Solution:

  1. Gracefully handle the edge case where an exec position might be intentionally unfilled, by simply omitting it from the Meet the Team page.
  2. Search through CSC members for both the current term and the previous term, when checking for execs. Note that this might make the build time slightly slower, since the script now needs to loop through two terms of members in order to pick out the execs, however the difference should be insignificant.

Staging: https://csclub.uwaterloo.ca/~a3thakra/csc/amy-news-2022-09-01

- Fixed edge case with fetching execs - Added Bootcamp mentor application news, and CSC x Google event **Problem:** When building the Meet the Team page using LDAP (in CI), we were only checking CSC members for the "current" term (Fall 2022) to see if any of them were execs. Since our actual current execs (the Spring 2022 execs, since Fall 2022 elections have not occurred yet) have not renewed their memberships for Fall 2022, the script was unable to find individuals for the Prez/VP/AVP/Trez positions, which caused an error. **Solution:** 1. Gracefully handle the edge case where an exec position might be intentionally unfilled, by simply omitting it from the Meet the Team page. 2. Search through CSC members for both the current term and the previous term, when checking for execs. Note that this might make the build time slightly slower, since the script now needs to loop through two terms of members in order to pick out the execs, however the difference should be insignificant. Staging: https://csclub.uwaterloo.ca/~a3thakra/csc/amy-news-2022-09-01
a258wang added 1 commit 5 months ago
c84b9e1cd8 Add Bootcamp mentor news and Google event
a258wang requested review from j285he 5 months ago
a258wang requested review from snedadah 5 months ago
a258wang requested review from b72zhou 5 months ago
a258wang requested review from e26chiu 5 months ago
a258wang removed review request for e26chiu 5 months ago
j285he reviewed 5 months ago
j285he left a comment
Collaborator

Sorry, aren't the images supposed to be square?

Sorry, aren't the images supposed to be square?
j285he approved these changes 5 months ago
j285he left a comment
Collaborator

Otherwise looks good

Otherwise looks good
j285he requested changes 5 months ago
j285he left a comment
Collaborator

Actually sorry, build is failing. hmm

Actually sorry, build is failing. hmm
Poster
Owner

Sorry, aren't the images supposed to be square?

There only seems to be a rectangular image for this event, and having a non-square image technically doesn't break anything, so I just included it lol.

> Sorry, aren't the images supposed to be square? There only seems to be a rectangular image for this event, and having a non-square image technically doesn't break anything, so I just included it lol.
Poster
Owner

Actually sorry, build is failing. hmm

The build error seems unrelated, but I'll look into it 🤔

(re-running CI didn't fix it)

image

> Actually sorry, build is failing. hmm The build error seems unrelated, but I'll look into it 🤔 (re-running CI didn't fix it) ![image](/attachments/6224c0fd-171f-45ba-9290-4b7107e776b7)
a258wang added 1 commit 5 months ago
5bd1890429 Add debug statement
a258wang added 1 commit 5 months ago
35dc436533 Fetch previous term execs on error
a258wang added 1 commit 5 months ago
d6b0767326 Fix parameters lol
a258wang added 1 commit 5 months ago
0b16c00780 Handle possibility of empty positions
a258wang added 1 commit 5 months ago
de03175e6f Add debugging statement
a258wang added 1 commit 5 months ago
017c44aaca Search last two terms for execs
a258wang force-pushed amy-news-2022-09-01 from 017c44aaca to 33b5baaff2 5 months ago
a258wang changed title from Add Bootcamp mentor news and Google event to Fix exec fetching and add events 5 months ago
a258wang requested review from j285he 5 months ago
Poster
Owner

The build failure was completely unrelated to my changes (our nightly builds actually started failing a couple days ago), but I've included the fix in this PR because screw the best practice of keeping PRs small and including related changes only it's not too big and also I got too lazy to make a separate PR. Sorry. 😛

The build failure was completely unrelated to my changes (our nightly builds actually started failing a couple days ago), but I've included the fix in this PR because ~~screw the best practice of keeping PRs small and including related changes only~~ it's not too big and also I got too lazy to make a separate PR. Sorry. 😛
j285he approved these changes 5 months ago
j285he left a comment
Collaborator

Looks good, can you just confirm that if you have person A as Pres for Spring, and person B as Pres for Fall, person B shows up?

Looks good, can you just confirm that if you have person A as Pres for Spring, and person B as Pres for Fall, person B shows up?
formattedExec = orderedExecPositions
.map((position) => {
const fullName = execMembers[position];
if (fullName == undefined) {
Collaborator

Please use === or specify if you want fullName === undefined | fullName === null etc

Please use `===` or specify if you want `fullName === undefined | fullName === null` etc
Poster
Owner

I remember Adi once said something about how it's okay to use == for checking equality with null or undefined, since in most cases there's no practical difference between null vs. undefined. (In this case, I think the variable would be undefined rather than null, but we'd still get an error if it was null, so we may as well apply the same logic to handle both cases and keep the conditional check concise instead of writing fullName === undefined || fullName === null.)

I remember Adi once said something about how it's okay to use `==` for checking equality with `null` or `undefined`, since in most cases there's no practical difference between `null` vs. `undefined`. (In this case, I think the variable would be `undefined` rather than `null`, but we'd still get an error if it was `null`, so we may as well apply the same logic to handle both cases and keep the conditional check concise instead of writing `fullName === undefined || fullName === null`.)
Poster
Owner

Looks good, can you just confirm that if you have person A as Pres for Spring, and person B as Pres for Fall, person B shows up?

With the way ceo (CSC's member management system thing) is set up, we only keep track of the current execs (not "execs for each term"), so yeah we'll display the Fall execs as soon as those are updated in the system (ie. right after Fall elections).

> Looks good, can you just confirm that if you have person A as Pres for Spring, and person B as Pres for Fall, person B shows up? With the way ceo (CSC's member management system thing) is set up, we only keep track of the current execs (not "execs for each term"), so yeah we'll display the Fall execs as soon as those are updated in the system (ie. right after Fall elections).
a258wang merged commit 94156adfd2 into main 5 months ago
a258wang deleted branch amy-news-2022-09-01 5 months ago

Reviewers

snedadah was requested for review 5 months ago
b72zhou was requested for review 5 months ago
j285he approved these changes 5 months ago
continuous-integration/drone/push Build is passing
The pull request has been merged as 94156adfd2.
Sign in to join this conversation.
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#491
Loading…
There is no content yet.