Parity/certifications#21
Conversation
…ty/certifications
| package io.github.tomhula.jecnaapi.data.cert | ||
|
|
||
| data class Certificate( | ||
| val dateIssued: String, |
| suspend fun getStudentProfile(username: String): Student | ||
| suspend fun getNotifications(): List<NotificationReference> | ||
| suspend fun getNotification(notification: NotificationReference): Notification | ||
| suspend fun getStudentCertificates(): List<Certificate>? |
There was a problem hiding this comment.
It's nullable because only fourth-graders have access to that URL.
There was a problem hiding this comment.
Your implementation never returns null. What happens if a non-fourth-grader calls this method? You left it totally unhandled. It will crash for anyone not in 4th grade
There was a problem hiding this comment.
Now it will check for the /neopravneny-pristup and throw an exception, but we will hide the certificate section in the app for non 4th graders via a check, so they wont get confused
|
Have you tested it on all teachers? (just a loop trying to fetch each teacher without any exceptions) |
|
On teachers that lack any certificate, it will return blank (nothing) |
|
it now uses LocalDate with your parser. |
|
Is the PR ready? |
| val infoSpan = li.selectFirst("span.info") | ||
| val label = infoSpan?.selectFirst("span.label")?.text()?.trim().orEmpty() | ||
| val institution = infoSpan?.selectFirst("span.institution")?.text()?.trim().orEmpty() | ||
| if (date.toString().isNotEmpty() && label.isNotEmpty()) |
There was a problem hiding this comment.
date.toString.isNotEmpty() will never be false. You are already parsing the date beforehand, which will throw an exception if it is empty. Also I would allow the empty label. I can imagine there being a certificate without a name, but with an institution. You can make it so both can't be empty at once, in which case it may be smarter to throw an exception, since it will probably be an error in the code. (as it is not expected there will actually be a certificate without both label and institution) Your CertificatePageParser implementation permits both being empty, so unify it somehow.
There was a problem hiding this comment.
now it allows the label to be empty
|
I have changed the handling. Please test everything out |
2 participants
Fix parity issue with Jecna App, user can now see teacher's certifications and their too.