Skip to content

Conversation

DavidN016
Copy link
Collaborator

-Created a post /edit, and a helper function for edit alias.
-Created edit mode in pencil/edit button, that edits the alias when clicked.
-Created test files for post /edit with dummy document.

DavidN016 added 2 commits September 19, 2025 22:28
- Implemented a new endpoint to edit card aliases, including validation for input.
- Updated the OfficeAccessCard utility to support alias editing.
- Enhanced the CardReader component to allow users to edit card aliases directly in the UI.
- Added corresponding tests to ensure proper functionality and error handling for the new feature.
- Introduced a new audit log action for alias edits.
Comment on lines 112 to 116
setCards(prevCards =>
prevCards.map(card =>
card._id === cardId
? { ...card, alias: editedAlias.trim() }
: card
Copy link
Contributor

Choose a reason for hiding this comment

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

i think what we should do here is when the alias is successfully edited, we query mongodb for all cards again, so that way this alias isn't only client side

Comment on lines 54 to 62
export function pencilSymbol(color = '#6b7280') {
return (
<svg width='20' height='20' viewBox='0 0 24 24' fill='none' stroke={color} strokeWidth='2' strokeLinecap='round' strokeLinejoin='round'>
<path d='M11 4H4a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h14a2 2 0 0 0 2-2v-7'/>
<path d='m18.5 2.5a2.121 2.121 0 0 1 3 3L12 15l-4 1 1-4 9.5-9.5z'/>
</svg>
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't have to be put in this file, the trashCanSymbol function is only imported from here because it was already implemented here when card reader page was made. you can move this to CardReader.js

Comment on lines 279 to 284
it('Should return 400 when both _id and alias are missing', async () => {
setTokenStatus(true);
const result = await test.sendPostRequestWithToken(token,
EDIT_API_PATH, {});
expect(result).to.have.status(BAD_REQUEST);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

this test is redundant

expect(updatedCard.alias).to.equal(NEW_ALIAS);
});
});

Copy link
Contributor

Choose a reason for hiding this comment

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

very impressive how thorough these tests are :)

Copy link
Collaborator

@evanugarte evanugarte left a comment

Choose a reason for hiding this comment

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

can there be some screenshots of the new ui

Comment on lines 220 to 223
const decoded = decodeToken(req);
if (!decoded) {
return res.sendStatus(UNAUTHORIZED);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we realized this doesnt work anymore, need to merge #1877 first

ill work on it

@adarshm11
Copy link
Contributor

can there be some screenshots of the new ui

image image

@adarshm11 adarshm11 changed the title Add edit alias functionality for OfficeAccessCard (#1900) OfficeAccessCard editing - UI changes Oct 18, 2025
@adarshm11 adarshm11 force-pushed the editalias branch 2 times, most recently from 003c142 to a3cec14 Compare October 18, 2025 17:51
@adarshm11 adarshm11 changed the title OfficeAccessCard editing - UI changes OfficeAccessCard editing - backend changes Oct 18, 2025
Comment on lines 227 to 231
const { _id, alias } = req.body;

if (!_id || !alias) {
return res.status(BAD_REQUEST).send('_id and alias are required in request body');
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should do this approach, from the verify endpoint, alias.trim() can also be in the array

  const required = [
    { value: apiKey, title: 'X-API-Key HTTP header', },
    { value: cardBytes, title: 'cardBytes query parameter', },
  ];
  const missingValue = required.find(({ value }) => !value);
  if (missingValue) {
    writeLogToClient(req.method, {
      statusCode: BAD_REQUEST,
      message: `${missingValue.title} missing from request`
    });
    return res.status(BAD_REQUEST).send(`${missingValue.title} missing from request`);
  }

Comment on lines 239 to 241
if (!/^[0-9a-fA-F]{24}$/.test(_id)) {
return res.status(BAD_REQUEST).send('_id must be a valid ObjectId');
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could even throw this in the missingFields thing

const updatedCard = await editAlias(_id, alias);

if (!updatedCard) {
return res.status(NOT_FOUND).send('Card not found');
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can just do sendStatus, 404 status implies that we didnt find it

Copy link
Contributor

Choose a reason for hiding this comment

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

done

action: AuditLogActions.EDIT_CARD,
details: {
newAlias: alias,
oldAlias: updatedCard.alias !== alias ? 'unknown' : alias
Copy link
Collaborator

Choose a reason for hiding this comment

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

why would updatedCard.alias not exist

also we could just do

details: {
  newAlias: alias,
  _id,
}

no need to say what the old one was

Copy link
Contributor

Choose a reason for hiding this comment

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

done

DavidN016 and others added 9 commits October 18, 2025 11:31
- Implemented a new endpoint to edit card aliases, including validation for input.
- Updated the OfficeAccessCard utility to support alias editing.
- Enhanced the CardReader component to allow users to edit card aliases directly in the UI.
- Added corresponding tests to ensure proper functionality and error handling for the new feature.
- Introduced a new audit log action for alias edits.
Comment on lines 84 to 90
if (!apiKey) {
writeLogToClient(req.method, {
statusCode: UNAUTHORIZED,
message: `Invalid API key: ${apiKey}`,
message: 'API key missing from request',
});
return res.sendStatus(UNAUTHORIZED);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we even need this? the required array already checks this i think

const { _id, alias } = req.body;

const required = [
{ value: _id && /^[0-9a-fA-F]{24}$/.test(_id) ? _id : null, title: 'Card ID', },
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe have the title be valid, alphanumeric Card ID

if (!result) {
const { description } = body;
logger.info(`Card:${description} not found in the database`);
logger.info('Card not found in the database');
Copy link
Collaborator

Choose a reason for hiding this comment

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

why did we change this line can we put it back

expect(result).to.have.status(BAD_REQUEST);
});

it('Should preserve other card properties when updating alias', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

up to you, should we add a quick test ensuring that only officers and above can edit a card, not member role

});

router.post('/edit', async (req, res) => {
const decoded = await decodeToken(req);
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we have to pass in access role officer here, otherwise anyone with a valid token at any role level can do this

@adarshm11 adarshm11 force-pushed the editalias branch 2 times, most recently from 3cee241 to a902e38 Compare October 20, 2025 04:30
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