Skip to content

Add UI for Card Management#1731

Merged
adarshm11 merged 13 commits into
devfrom
Adarsh/06252025_card_management_ui
Jul 7, 2025
Merged

Add UI for Card Management#1731
adarshm11 merged 13 commits into
devfrom
Adarsh/06252025_card_management_ui

Conversation

@adarshm11

Copy link
Copy Markdown
Contributor

Add UI to manage/delete cards, move CardReader API functions to separate file, fix Messaging bug

@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch 2 times, most recently from e6c91f6 to 6099b92 Compare June 28, 2025 23:59
@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch from 6099b92 to a5003d9 Compare June 29, 2025 00:01
, (error, result) => {
if (error) {
logger.error('getAllCards got an error querying mongodb');
return resolve(null);

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.

maybe we should resolve an empty array, so this function returns some sort of array no matter what

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 +211 to +218
let getCards = await getAllCards();
if (!getCards) {
logger.info('Error retrieving cards');
return res.sendStatus(SERVER_ERROR);
}

logger.info('Retrieved all cards successfully!');
res.json(getCards).status(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.

this is a silly nitpick, the less we add not in our if statements the easier debugging becomes later

also we already log errors, so we can skip logging it again, plus logging nothing on success is ok

Suggested change
let getCards = await getAllCards();
if (!getCards) {
logger.info('Error retrieving cards');
return res.sendStatus(SERVER_ERROR);
}
logger.info('Retrieved all cards successfully!');
res.json(getCards).status(OK);
const getCards = await getAllCards();
if (getCards) {
return res.json(getCards).status(OK);
}
return res.sendStatus(SERVER_ERROR);

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 +29 to +31
const url = new URL('/api/OfficeAccessCard/delete', BASE_API_URL);
url.searchParams.append('cardBytes', cardBytes);
const res = await fetch(url.href, {

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 didnt do this earlier but can we change the delete endpoint to be a post request? then we can send the bytes as a json body

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 thread src/Pages/CardReader/CardReader.js Outdated
Comment thread src/Pages/CardReader/CardReader.js Outdated
Comment thread api/main_endpoints/routes/OfficeAccessCard.js Outdated
Comment thread src/Pages/CardReader/CardReader.js Outdated
Comment thread src/Pages/CardReader/CardReader.js Outdated
>
Card Registry
</button>
<div>&nbsp;|&nbsp;</div>

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.

add a comment above this line on what this is rendering

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.

also can we get screenshots of the new ui, with how both tabs look

body: JSON.stringify({ cardBytes, }),
});
if (res.ok) {
const result = await res.json();

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 json response from the delete endpoint?

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.

i believe its a boolean of success/failure

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.

double check if this res.json() returns an error

Comment thread src/Pages/CardReader/CardReader.js
Comment thread src/Pages/CardReader/CardReader.js Outdated
headerText: `Delete card: ${cardToDelete.cardBytes}?`,
bodyText: `Are you sure you want to delete
card: ${cardToDelete.cardBytes}? It'll be gone forever if you do.`,
confirmText: `Yes, delete card: ${cardToDelete.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.

Suggested change
confirmText: `Yes, delete card: ${cardToDelete.cardBytes}`,
confirmText: `Yes, delete ${cardToDelete.cardBytes}`,

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

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 the original is still there

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.

I haven't committed yet, still working on the useMemo thing below

Comment thread src/Pages/CardReader/CardReader.js Outdated
Comment thread src/Pages/CardReader/CardReader.js Outdated
Comment thread src/Pages/CardReader/CardReader.js Outdated
@adarshm11

Copy link
Copy Markdown
Contributor Author

also can we get screenshots of the new ui, with how both tabs look

image image

@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch 4 times, most recently from edb258c to eb05d9a Compare June 29, 2025 19:47
@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch from eb05d9a to 91a097c Compare June 29, 2025 19:48
@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch from 6f83717 to 0935119 Compare July 4, 2025 16:43
@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch from a636313 to 7e0e19c Compare July 4, 2025 16:46
endpoint: '/delete',
message: 'cardBytes missing from request'
message: 'cardBytes missing from request',
requestType: 'HTTP POST',

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 get the method from the request object?

Comment thread src/Pages/CardReader/CardReader.js Outdated
eventSource.onerror = (error) => {
setLogs(
(currLogs) => [
'...crickets...',

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.

instead can we show the error in the console

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.

I can log the error in the browser console but it'll not pass lint and also won't actually say what happened, it's a very vague Event object


useEffect(() => {
if (connected) {
setConnectionStatusText('Listening for card reader activity...');

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
setConnectionStatusText('Listening for card reader activity...');
setConnectionStatusText('Listening for card reader activity...');
return;

then you can remove the else block

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/06252025_card_management_ui branch from c455f42 to a03cd58 Compare July 6, 2025 20:18
@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch from 4cbaeb8 to 8d74c78 Compare July 7, 2025 05:09

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

big

@adarshm11
adarshm11 force-pushed the Adarsh/06252025_card_management_ui branch from eb5e9e5 to a7a5b61 Compare July 7, 2025 05:35
Comment thread api/main_endpoints/routes/OfficeAccessCard.js Outdated
Comment thread api/main_endpoints/routes/OfficeAccessCard.js Outdated
Comment thread api/main_endpoints/routes/OfficeAccessCard.js Outdated
Co-authored-by: Evan Ugarte <36345325+evanugarte@users.noreply.github.com>
@adarshm11
adarshm11 merged commit e90333d into dev Jul 7, 2025
4 checks passed
@adarshm11
adarshm11 deleted the Adarsh/06252025_card_management_ui branch July 7, 2025 05:45
aeiouny pushed a commit to aeiouny/Clark that referenced this pull request Jul 7, 2025
* Add UI to manage cards, move CardReader API functions to separate file, fix Messaging bug

* Addressed previous comments

* Addressed comments

* addressed more comments

* added light mode compatibility, adjusted log format, set default card verifiedCount to 1

* Add pagination to card registry

* Removed search and sort logic for now

* Add pagination to card registry

* Removed search and sort logic for now

* fix crickets msg

* Apply suggestions from code review

Co-authored-by: Evan Ugarte <36345325+evanugarte@users.noreply.github.com>

---------

Co-authored-by: Evan Ugarte <36345325+evanugarte@users.noreply.github.com>
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