Skip to content

Change ui on led sign page to have a tab, if the user is an officer or admin#1988

Merged
evanugarte merged 9 commits into
devfrom
requestUI4admins
Mar 9, 2026
Merged

Change ui on led sign page to have a tab, if the user is an officer or admin#1988
evanugarte merged 9 commits into
devfrom
requestUI4admins

Conversation

@wayngo

@wayngo wayngo commented Jan 3, 2026

Copy link
Copy Markdown
Collaborator
image

@wayngo
wayngo force-pushed the Request-Button4LEDSIGN branch from 86a5910 to 0572c22 Compare January 3, 2026 06:42
Base automatically changed from Request-Button4LEDSIGN to LED4MEMBERS January 3, 2026 08:55
@wayngo
wayngo changed the base branch from LED4MEMBERS to Request-Button4LEDSIGN January 3, 2026 09:12
@wayngo

wayngo commented Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author
image

Comment on lines +17 to +20
approved: {
type: Boolean,
default: false,
},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets use an enum here, like PENDING, APPROVED, DENIED

Comment on lines +28 to +36
if (existingRequest) {
if (existingRequest.status === PermissionRequestStatus.PENDING) {
return res.sendStatus(CONFLICT);
}
existingRequest.status = PermissionRequestStatus.PENDING;
existingRequest.createdAt = new Date();
await existingRequest.save();
return res.sendStatus(OK);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think if it exists we just return conflict no matter what, what do you think

if its denied, we can set the status to pending

Comment on lines +89 to +91
if (!type || !Object.keys(PermissionRequestTypes).includes(type)) {
return res.status(BAD_REQUEST).send({ error: 'Invalid type' });
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing like we did before can we do

`type ${type} is invalid, valid types are ${Object.keys(PermissionRequestTypes)}`

Comment thread src/APIFunctions/PermissionRequest.js Outdated
Comment on lines +48 to +57
if (res.ok) {
// API returns 200 with no body, so we just mark success
status.responseData = true;
} else if (res.status === 409) {
// CONFLICT - duplicate request
status.error = true;
status.responseData = 'Request already exists';
} else {
status.error = true;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (res.ok) {
// API returns 200 with no body, so we just mark success
status.responseData = true;
} else if (res.status === 409) {
// CONFLICT - duplicate request
status.error = true;
status.responseData = 'Request already exists';
} else {
status.error = true;
}
status.error = !!res.ok;
if (res.status === 409) {
// CONFLICT - duplicate request
status.responseData = 'Request already exists';
}

Comment thread src/APIFunctions/PermissionRequest.js Outdated
});

if (res.ok) {
status.responseData = true;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets not even set responseData

for these create/delete requests we can just rely on if error is true/false for checking if it worked

Base automatically changed from Request-Button4LEDSIGN to dev March 9, 2026 03:08

@weslayer weslayer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W growtopia player

@evanugarte
evanugarte merged commit 26d91c6 into dev Mar 9, 2026
4 checks passed
@evanugarte
evanugarte deleted the requestUI4admins branch March 9, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants