www
/
www-new
12
3
Fork 5
Code Issues 58 Pull Requests 42 Projects Releases Wiki Activity

Fix exec fetching and add events #491

Merged
a258wang merged 7 commits from amy-news-2022-09-01 into main 2022-09-02 14:34:04 -04:00
Conversation 4 Commits 7 Files Changed 5 +74 -23
a258wang commented 2022-09-01 17:48:11 -04:00
Owner
Copy Link
  • 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 2022-09-01 17:48:13 -04:00
continuous-integration/drone/push Build is failing Details
c84b9e1cd8 Add Bootcamp mentor news and Google event
a258wang requested review from j285he 2022-09-01 17:48:26 -04:00
a258wang requested review from snedadah 2022-09-01 17:48:26 -04:00
a258wang requested review from b72zhou 2022-09-01 17:48:26 -04:00
a258wang requested review from e26chiu 2022-09-01 17:48:26 -04:00
a258wang removed review request for e26chiu 2022-09-01 17:48:35 -04:00
j285he reviewed 2022-09-01 20:06:27 -04:00
j285he left a comment
Member
Copy Link

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

Sorry, aren't the images supposed to be square?
j285he approved these changes 2022-09-01 20:06:49 -04:00
j285he left a comment
Member
Copy Link

Otherwise looks good

Otherwise looks good
j285he requested changes 2022-09-01 20:07:45 -04:00
j285he left a comment
Member
Copy Link

Actually sorry, build is failing. hmm

Actually sorry, build is failing. hmm
a258wang commented 2022-09-01 22:33:08 -04:00
Author
Owner
Copy Link

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.
a258wang commented 2022-09-01 22:39:15 -04:00
Author
Owner
Copy Link

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)
image.png
62 KiB
a258wang added 1 commit 2022-09-01 23:32:27 -04:00
continuous-integration/drone/push Build is failing Details
5bd1890429 Add debug statement
a258wang added 1 commit 2022-09-02 00:33:19 -04:00
continuous-integration/drone/push Build is failing Details
35dc436533 Fetch previous term execs on error
a258wang added 1 commit 2022-09-02 00:37:21 -04:00
continuous-integration/drone/push Build is passing Details
d6b0767326 Fix parameters lol
a258wang added 1 commit 2022-09-02 01:02:55 -04:00
continuous-integration/drone/push Build is passing Details
0b16c00780 Handle possibility of empty positions
a258wang added 1 commit 2022-09-02 01:03:39 -04:00
continuous-integration/drone/push Build is passing Details
de03175e6f Add debugging statement
a258wang force-pushed amy-news-2022-09-01 from 017c44aaca to 33b5baaff2 2022-09-02 01:36:21 -04:00 Compare
a258wang changed title from Add Bootcamp mentor news and Google event to Fix exec fetching and add events 2022-09-02 01:37:57 -04:00
a258wang requested review from j285he 2022-09-02 01:55:50 -04:00
a258wang commented 2022-09-02 02:00:40 -04:00
Author
Owner
Copy Link

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 2022-09-02 02:36:26 -04:00
j285he left a comment
Member
Copy Link

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?
lib/team.ts
@ -128,0 +134,4 @@
formattedExec = orderedExecPositions
.map((position) => {
const fullName = execMembers[position];
if (fullName == undefined) {
j285he commented 2022-09-02 02:33:10 -04:00
Member
Copy Link

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

Please use `===` or specify if you want `fullName === undefined | fullName === null` etc
a258wang commented 2022-09-02 14:26:59 -04:00
Author
Owner
Copy Link

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`.)
👍 1
a258wang commented 2022-09-02 14:33:34 -04:00
Author
Owner
Copy Link

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).
👍 1
a258wang merged commit 94156adfd2 into main 2022-09-02 14:34:04 -04:00
a258wang deleted branch amy-news-2022-09-01 2022-09-02 14:34:05 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
snedadah
b72zhou
j285he
Labels
Clear labels   
a11y

   
Backlog

   
Blocked

   
Bug

   
Content

   
Dependencies

   
Design

   
Feature Request

   
Good First Issue

   
In Progress

   
Performance

   
Priority - High

   
Priority - Low

   
Priority - Medium

   
Untriaged
No Label
a11y
Backlog
Blocked
Bug
Content
Dependencies
Design
Feature Request
Good First Issue
In Progress
Performance
Priority - High
Priority - Low
Priority - Medium
Untriaged
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#491
No description provided.
Delete Branch "amy-news-2022-09-01"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?