Use LDAP API to fetch executive members' name (Closes #376) #396

Merged
b72zhou merged 42 commits from b72zhou-ldap-exec into main 2022-03-16 02:17:34 -04:00
8 changed files with 112 additions and 19 deletions

View File

@ -1,11 +1,32 @@
import { readFile, readdir, access } from "fs/promises"; import { readFile, access } from "fs/promises";
import path from "path"; import path from "path";
import matter from "gray-matter"; import matter from "gray-matter";
import { Client } from "ldapts";
import { serialize } from "next-mdx-remote/serialize"; import { serialize } from "next-mdx-remote/serialize";
import { getCurrentTerm } from "@/lib/events";
import { capitalize } from "@/utils";
const EXECS_PATH = path.join("content", "team", "execs"); const EXECS_PATH = path.join("content", "team", "execs");
b72zhou marked this conversation as resolved Outdated

Can we consider changing this to FILETYPE since this constant is not supposed to change?

Can we consider changing this to `FILETYPE` since this constant is not supposed to change?
const fileType = ".md"; const FILETYPE = ".md";
const { year, term } = getCurrentTerm();
const execPositions: { [position: string]: string } = {
president: "President",
"vice-president": "Vice President",
secretary: "Assistant Vice President",
treasurer: "Treasurer",
sysadmin: "System Administrator",
};
n3parikh marked this conversation as resolved Outdated

Turn this into a object that maps from the LDAP short name (ex. sysadmin) and then index by that instead. We shouldn't index by the position order, since that could change.

ie.

{
	"president": "President",
    ...
}
Turn this into a object that maps from the LDAP short name (ex. `sysadmin`) and then index by that instead. We shouldn't index by the position order, since that could change. ie. ``` { "president": "President", ... } ```
const orderedExecPositions: string[] = [
b72zhou marked this conversation as resolved Outdated

Could we call this array orderedExecPositions or something? I was a little confused by why we needed this array at first until I realized that we use it to enforce the order of execs.

Could we call this array orderedExecPositions or something? I was a little confused by why we needed this array at first until I realized that we use it to enforce the order of execs.
"president",
"vice-president",
"secretary",
"treasurer",
"sysadmin",
];
b72zhou marked this conversation as resolved Outdated

Should this be is ExecMember (singular), since it represents only one exec?

Should this be is `ExecMember` (singular), since it represents only one exec?
export interface Metadata { export interface Metadata {
name: string; name: string;
@ -13,23 +34,93 @@ export interface Metadata {
image: string; image: string;
} }
export async function getExecNames() { export async function getExecNamePosPairs() {
return (await readdir(EXECS_PATH)) if (process.env.USE_LDAP?.toLowerCase() !== "true") {
.filter((name) => name.endsWith(fileType)) return [["codey", "mascot"]];
.map((name) => name.slice(0, -1 * fileType.length)); }
const url = "ldap://ldap1.csclub.uwaterloo.ca";
const searchDN = "ou=People,dc=csclub,dc=uwaterloo,dc=ca";
const client = new Client({ url });
// position: name
b72zhou marked this conversation as resolved Outdated

If I'm understanding the code correctly, could we change the type to { [position: string]: string } so that it's inline with the documentation above? If you like this change, also change line 14

If I'm understanding the code correctly, could we change the type to `{ [position: string]: string }` so that it's inline with the documentation above? If you like this change, also change line 14
const execMembers: { [position: string]: string } = {};
let formattedExec: [string, string][] = [];
try {
await client.bind("", "");
const { searchEntries } = await client.search(searchDN, {
scope: "sub",
filter: `(&(objectClass=member)(term=${(term as string).slice(
0,
1
)}${year}))`,
});
// item.position might be an array if the member has more than one position
searchEntries.forEach((item) => {
if (typeof item.position === "string" && item.position in execPositions) {
execMembers[item.position] = item.cn as string;
} else if (item.position instanceof Array) {
item.position.forEach((p) => {
b72zhou marked this conversation as resolved
Review

Why are you checking if it is an object, rather than an array?

Why are you checking if it is an object, rather than an array?
Review

Whoops, that was because when I use typeof to get the type of an array, it returned "object". But now I found that instanceof would return "array" type direcly. Let me change that. :)

Whoops, that was because when I use typeof to get the type of an array, it returned "object". But now I found that instanceof would return "array" type direcly. Let me change that. :)
if ((p as string) in execPositions) {
execMembers[p as string] = item.cn as string;
}
});
}
});
formattedExec = orderedExecPositions.map((position) => {
return [

We shouldn't rely on the position we want to be first. Also, this is unlikely, but it is possible that one person has mutiple postions we care about.

Here is what I propose instead (in psuedocode):

let positions = ["president", "vice-president", ...]
let execMembers = {}
// Write a comment here explaining that item.position may be an array
searchEntries.forEach item
	if item.position is string and item.position in positions:
    	execMembers[item.position] = { construct ExecMember object }
    else if item.position is array:
    	item.position.forEach p
        	if p in positions:
            	execMembers[p] = { construct ExecMember object }

execMembers = positions.map((position) => execMembers[position]);

You can get rid of the sort afterwards and also the execPositions object, since the positions array will be in the correct order.

We shouldn't rely on the position we want to be first. Also, this is unlikely, but it is possible that one person has mutiple postions we care about. Here is what I propose instead (in psuedocode): ``` let positions = ["president", "vice-president", ...] let execMembers = {} // Write a comment here explaining that item.position may be an array searchEntries.forEach item if item.position is string and item.position in positions: execMembers[item.position] = { construct ExecMember object } else if item.position is array: item.position.forEach p if p in positions: execMembers[p] = { construct ExecMember object } execMembers = positions.map((position) => execMembers[position]); ``` You can get rid of the sort afterwards and also the `execPositions` object, since the positions array will be in the correct order.
`${execMembers[position].split(" ")[0].toLowerCase()}-${execMembers[
position
]
.split(" ")[1]
.toLowerCase()}`,
position,
];
});
formattedExec = [...formattedExec, ["codey", "mascot"]];
} finally {
await client.unbind();
}
return formattedExec;
} }
export async function getExec(fileName: string, convert = true) { export async function getExec(name: string, pos: string, convert = true) {
const raw = await readFile(path.join(EXECS_PATH, `${fileName}${fileType}`)); let content, metadata;
const { content, data: metadata } = matter(raw);
const image =
(metadata.image as string | undefined) ??
(await getMemberImagePath(metadata.name));
return { try {
content: convert ? await serialize(content) : content, const raw = await readFile(path.join(EXECS_PATH, `${name}${FILETYPE}`));
metadata: { ...metadata, image } as Metadata, ({ content, data: metadata } = matter(raw));
};
const image = await getMemberImagePath(metadata.name);
return {
content: convert ? await serialize(content) : content,
metadata: { ...metadata, image } as Metadata,
};
} catch (err) {
// Capitalize the first letter of the first name and last name
const firstName = capitalize(name.split("-")[0]);
const lastName = capitalize(name.split("-")[1]);

Please leverage the capitalize function in utils.ts

Please leverage the capitalize function in `utils.ts`
const posName = execPositions[pos];

I think it's not ideal that we use a position number, since it means another thing we need to keep in sync.

What do you think of returning a list of file-name, position pairs in getExecNames, and then dropping the number from the file names? Ie. instead of ["01-juthika-hoque.md", "02-eric-huang.md", ...], we return [["juthika-hoque.md", "president"], ...]., and then pass that pair to getExec. Then on line 111, you can index by the second entry in the pair, instead of positionNames[Number(posOrder) - 1].

I think it's not ideal that we use a position number, since it means another thing we need to keep in sync. What do you think of returning a list of file-name, position pairs in `getExecNames`, and then dropping the number from the file names? Ie. instead of `["01-juthika-hoque.md", "02-eric-huang.md", ...]`, we return `[["juthika-hoque.md", "president"], ...]`., and then pass that pair to `getExec`. Then on line 111, you can index by the second entry in the pair, instead of `positionNames[Number(posOrder) - 1]`.

Great idea I think! I've commited the code that implement this and we can discuss it tmr :))

Great idea I think! I've commited the code that implement this and we can discuss it tmr :))
content = "Coming Soon!";
metadata = {
name: `${firstName} ${lastName}`,
role: `${posName}`,
};
const image = await getMemberImagePath(metadata.name);
return {
content: convert ? await serialize(content) : content,
metadata: { ...metadata, image } as Metadata,
};
}
} }
async function getImage(imgPath: string) { async function getImage(imgPath: string) {

View File

@ -11,7 +11,7 @@ import { TeamMemberCard } from "@/components/TeamMemberCard";
import { Title } from "@/components/Title"; import { Title } from "@/components/Title";
import { import {
getExec, getExec,
getExecNames, getExecNamePosPairs,
Metadata, Metadata,
getMemberImagePath, getMemberImagePath,
} from "@/lib/team"; } from "@/lib/team";
@ -208,10 +208,12 @@ async function getTeamWithImages(team: TeamMember[]) {
} }
export const getStaticProps: GetStaticProps<Props> = async () => { export const getStaticProps: GetStaticProps<Props> = async () => {
const execNames = await getExecNames(); const execNamePosPairs = await getExecNamePosPairs();
const execs = (await Promise.all( const execs = (await Promise.all(
execNames.map((name) => getExec(name)) execNamePosPairs.map((namePosPair) =>
getExec(namePosPair[0], namePosPair[1])
)
)) as SerializedExec[]; )) as SerializedExec[];
const [ const [