Skip to content

Create Card Reader page for admin UI#1652

Merged
adarshm11 merged 14 commits into
devfrom
Adarsh/05272025_card_reader_ui
Jun 20, 2025
Merged

Create Card Reader page for admin UI#1652
adarshm11 merged 14 commits into
devfrom
Adarsh/05272025_card_reader_ui

Conversation

@adarshm11

Copy link
Copy Markdown
Contributor

I created a card reader page only visibly to admins/officers for Step 2 of the SCE Badge Reader project

Comment on lines +91 to +93
router.get('/listen', (req, res) => {
res.setHeader('Content-Type', 'text/event-stream');
res.setHeader('Cache-Control', 'no-cache');

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.

can we add verification to this api, where the user has to have a valid token

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +50 to +56
const response = {
endpoint: '/verify',
statusCode: 200,
message: 'Card authorized!'
};

const writeRequestResponse = () => {

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.

response shouldnt be global, instead something you initialize before you call writeRequestResponse and pass as a parameter

Suggested change
const response = {
endpoint: '/verify',
statusCode: 200,
message: 'Card authorized!'
};
const writeRequestResponse = () => {
const writeRequestResponse = (response) => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


const cardExists = await checkIfCardExists(cardBytes);
if (cardExists) {
response.endpoint = '/verify'; // if user tried to add a card that already existed, ignore the "add" parameter

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.

we dont need this line?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@evanugarte evanugarte 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.

i think we should call writeRequestResponse with the object that we want to send back to the client, like

const defaultResponse = {
  cardWasAdded: false,
  message: null,
  endpoint: '/verify',
};

const writeRequestResponse = ({ statusCode, ...rest }) => {
  const response = {
    statusCode,
    ...defaultResponse,
    ...rest, // override any default values
  };

  clients.forEach(client => {
    client.res.write(`data: ${JSON.stringify(response)}\n\n`);
  });
};

that way we can call on it like

writeRequestResponse({ statusCode: OK });
writeRequestResponse({ statusCode: OK, cardWasAdded: true });
writeRequestResponse({ statusCode: NOT_FOUND });
writeRequestResponse({
  statusCode: BAD_REQUEST,
  message: `${missingValue.title} missing from request`,
});

}
});

router.get('/delete', async (req, res) => {

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 move this to another pr? something that would be merged after this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

});
}

function deleteCard(cardBytes) {

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.

this can also be in another pr

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@adarshm11 adarshm11 force-pushed the Adarsh/05272025_card_reader_ui branch from ef5f60f to f087c18 Compare June 19, 2025 04:50
Comment thread src/Pages/CardReader/CardReader.js Outdated
Comment on lines +10 to +13
let endpoint = data.endpoint;
if (!endpoint.includes('?add=1')) {
endpoint += ' ';
}

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 we can use the field cardWasAdded?

Comment thread src/Pages/CardReader/CardReader.js Outdated
if (!endpoint.includes('?add=1')) {
endpoint += ' ';
}
return [new Date().toISOString(), endpoint, data.statusCode, data.message].join(' ');

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.

up to you, what if we use '\t' instead? or does it not look as good

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

\t is inconsistent for some reason so I used padEnd() instead

Comment thread src/Pages/CardReader/CardReader.js Outdated
Comment on lines +22 to +24
let data = JSON.parse(event.data);
let newLog = buildLog(data);
setLogs(currLogs => [newLog, ...currLogs]); // prepend the new log

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 put a try/catch in here, so if for some reason the data cant be parsed we do

try {
      let data = JSON.parse(event.data);
      let newLog = buildLog(data);
      setLogs(currLogs => [newLog, ...currLogs]);
} catch (e) {
      console.error('unable to format event data from /listen', e);
      setLogs(
            (currLogs) => [
                  '[error] unable to format response, check browser logs',
                  ...currLogs,
            ]
      );
}

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.

also the above code likely needs to be linted

await new OfficeAccessCard({
cardBytes
}).save();
writeRequestResponse({ statusCode: OK, message: 'Card added!', endpoint: '/verify?add=1' });

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.

is there a way to dynamically get the endpoint from req?

@n8thantran n8thantran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

great job adarsh! lgtm

@adarshm11 adarshm11 merged commit 6f50df8 into dev Jun 20, 2025
4 checks passed
@adarshm11 adarshm11 deleted the Adarsh/05272025_card_reader_ui branch June 20, 2025 20:35
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