feat: handle multiple identical ids

[taavilooke]

Handle the case where there are multiple entries for identical ids in the database. Previously the code gave an error in this case and showed this as no card in the front end. Since instructors can add cards themselves now, we need to handle this case.

Added logic to find the highest value card that is currently valid and show that in the front-end.

Also fixed a bug that showed a valid card with no expiry date if no expiry date was set in the database. Now it shows as no card.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
climbing-card-check	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 14, 2024 11:13am

[taavilooke]

Found two errors:

1) The PR removes the line that returns formFillTime from findById function. It seemed that the UI doesn't use it anywhere so there's no reason to hand it over to the UI. However the tests do expect it right now. Let's remove it from the tests as well. 2) Testcase 40000000000 in the current implementation returns a certificate: "none". But in the PR's implementation it returns "error: not found". I think this is a more appropriate response, since the certificate is not correct.

Rauno, what do you think?

[rauno56]

To reply to your points:

1. Thing to keep in mind is that our UI is not the only consumer for the API. However with formFillTime... it probably is not used anywhere maybe?
2. I think that's fair because we control what's inserted in the registry. It would be useful to know whether one is dealing with invalid data or really-not-found tho.

• [ ] Fix the linting errors (I pushed some rules I noticed your code not following, but rest of the codebase having, so linter will guide you through it)
• [ ] Fix broken tests
• [ ] Add a test that tests the added case. fixture.txt is just copy-paste of the test rows in the doc so feel free to add those cases there first and copy the result from there without editing the fixture.txt directly.

[taavilooke]

• Should we rebase the whole branch to add conventioncommits style messages to all the commits?
• Should we clean up the history somehow with the merges? How to do that?

[rauno56]

Should we rebase the whole branch to add conventioncommits style messages to all the commits?
Should we clean up the history somehow with the merges? How to do that?

We should squash-merge PRs that include one feature like this one, so the individual commit messages do not matter. Only the PR title which will be used for the commit that lands in the main.

[taavilooke]

Right now it's meant to work so that if the red card is expired the valid green will be shown. However in practice the red card will always have a later expiration date than the green card and when the red card expires the green card will have expired earlier than that. Is this what you meant?

On Sun, Apr 21, 2024 at 8:27 PM Rauno Viskus @.***> wrote:

@.**** approved this pull request.

How are we handling a case when someones expired red will be replaced by valid green?

If that's handled (or knowingly left unhandled) correctly, feel free to squash-merge this.

— Reply to this email directly, view it on GitHub https://github.com/rauno56/climbing-card-check/pull/29#pullrequestreview-2013485544, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG2OPM34LIWM5M2ZFZIDOMDY6PZILAVCNFSM6AAAAABCYYHLFGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJTGQ4DKNJUGQ . You are receiving this because you authored the thread.Message ID: @.***>