Removed unnecessary text under verified studentstatus #4487
Conversation
| <Button success onClick={() => performStudentAuth()}> | ||
| Verifiser med FEIDE | ||
| </Button> |
There was a problem hiding this comment.
Maybe this should be hidden when verified? @ivarnakken @magnusbrecke
There was a problem hiding this comment.
Yes that would be reasonable. The FEIDE knapp has no purpose right now the way I see it. What do you think Ivar?
There was a problem hiding this comment.
No users have to verify themselves roughly every year if I remember correctly. So a user should definitely be able to verify whenever they want, especially if there are updates in their FEIDE user info, but perhaps we can change the wording to "Verifiser med FEIDE på nytt" or something?
There was a problem hiding this comment.
This is basically what the info text you removed said - so we should maybe update the text in the blue info card above slightly so that it is clear to the user that they are currently verified and that they can verify again if they want to (or need to, I guess).
There was a problem hiding this comment.
I'm assuming the feide-implementation is still only putting people in 1st/4th grade if they are not in a grade as this linear issue is still open: https://linear.app/abakus-webkom/issue/ABA-534/semi-auto-grade-selection-for-feide-verification
And if that's the case, wouldn't it be fine to hide the button? Currently re-verifying will not make any type of functional changes on the webapp (afaik), so I see no reason to encourage users to re-verify as of now.
Although if we get to a point where feide-verification will actually do something useful (other than avoiding having to check your studmail) (or if we are actually at that point right now and I'm just unaware), I would suggest the following;
{isVerified && <SuccessCard />} // title: Du er verifisert som student
{!isVerified && <DangerCard />} // title: Du er ikke verifisert som student
{isVerified && <WarningCard />}
/* title: Ønsker du å endre trinn eller studie?
Trykk på "Verifiser med FEIDE" for å hente nye opplysninger.
Fungerer ikke dette? Send en mail til webkom@abakus.no, ...resten av beskrivelsen av krav...*/
<VerifyButton>Verifiser med FEIDE</VerifyButton>Soo for now I'm positive to remove the button if you're already verified. And maybe splitting the InfoCard that we currently use into a SuccessCard and a WarningCard as in my mock example
There was a problem hiding this comment.
And regarding re-verifying every year.. If we at some point actually implement that (right now the feide-verification does not impact any of your rights on the website - the only "useful" part of it is that you get added to one of the student groups), it shouldn't say "Du er verifisert som student" if you have to re-verify.. Then it should say something like "Du må verifisere student-statusen din på nytt" in some sort of a danger-card style.
And of course make the button visible again - unless the semi-auto-grade selection is implemented.
There was a problem hiding this comment.
Yes that's true - we should definitely make the verification useful ..
There was a problem hiding this comment.
Just to slide in some background here. You are right on that when you are verified, you don't really need to reverify ever, except in some cases:
You changed from komtek <-> data. I believe (although I'm not 💯% sure) that I added logic for changing you study using the verification.
Its also here because when is student==false, the verification is done, but unsuccessfully, and then you have the option to try again later when studweb is correct or whatever. The text under the button was thought to explain why it's there so people don't get super confused, although I see how it might do the opposite. I also left it there when you are successfully verified because of the same reasons above I believe. I.e removing the button would remove the option to try again, making it possible to end up in a state where you user verification status is broken/stuck.
magnusbrecke
added
small-fix
There was a problem hiding this comment.
Great work! I think we should probably still show some confirmation that you're already verified (assuming we don't).
This is a bit unrelated but branch names should be all lower case; written in kebab case :)
PR title and branch name should also be in imperative, so something like "fix-student-verification" perhaps.
Again, this is unrelated to the actual work but makes the project overall more concise and adhere to standards :)
|
Awesome work 🤩 |
Related to this, I suggest reading the CONTRIBUTING.md |
|
@ivarnakken what is the conclusion on this pr? |
Like the others point out, we should still say that the user can verify themselves again to update their status. They don't necessarily have to write a mail to us |
Description
Removed unnecessary text under verified studentstatus since it was basically the same information as the box over, and it looked very untidy
Result
Before:
After:
Testing
Please describe what and how the changes have been tested, and provide instructions to reproduce if necessary.
Resolves ... ABA-641