feat: add issuer specific fip validity to operator pages#684
feat: add issuer specific fip validity to operator pages#684MoritzWeber0 wants to merge 18 commits intofeat/add-dialogsfrom
Conversation
✅ Deploy Preview for fipguide ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Should be ready for review now, @lenderom @therobrob. I'll keep it in the draft state until #680 is merged as this PR is based on it and should not be merged before. |
|
@MoritzWeber0 Thank you for your implementation. I think this will help many of our collegues. Do you think we should implement a kind of "Default" per employer? Like usually SNCF gives 1 Coupon with 4 Fields and we only specify the deviations on the operator site? |
I'm against an implicitly added default. We should only add explicit information if we have it, otherwise let's fall back to unknown. The only reason would be less boilerplate (the YAML section grows a bit too quickly maybe). But there are probably better solutions like defining some defaults globally and then just reference them via a one-liner. |
Yeah I was more thinking about the boilerplate, not a "default" information but a way to avoid writing on every page "SBB employees have 8 fields every field is valid for one day" :D |
|
Another point I was thinking about was the considerationfo things like #681 It doesn't have to be implemented in this PR, I was just thinking about covering this problem in your implementation. For example instead of having a row in the table for the FIP Coupon there could be a text like "unlimited free travel". But we could also find other ways of covering this topic :) |
This is the reason why I removed some of the structured data from the previous PR and made it more flexible with the text :) |
There was a problem hiding this comment.
I’ve just reviewed the technical parts, not the md-files.
Two further comments:
- for me it seems slightly confusing showing the grey status with question marks for cases, where we don’t know the information from all operators. It suggests we have no information at all. I think this case is very common, until we don’t have the information from all operators.
Edit: why do we handle the same case ("we don’t have information from all operators") in two different ways?
One time we say "valid" and one time we say "unknown" – where’s the difference between them?
- this leads me to the second aspect: the button for the dialog isn’t optimal in my opinion. It is not self-explanatory, because there is no label and there is the second status-icon to the left (two icons, two different meanings for interacting). I envision a single, comprehensive button, so the user can click the whole information.
| fipValidity: | ||
| additional: Sonstige | ||
| fip-coupon: FIP Freifahrtschein | ||
| fip-coupon-relatives: FIP Freifahrt Angehörige |
There was a problem hiding this comment.
| fip-coupon-relatives: FIP Freifahrt Angehörige | |
| fip-coupon-relatives: FIP Freifahrtschein für Angehörige |
| </div> | ||
|
|
||
| {{- if not .disable_dialog -}} | ||
| {{- partial "dialog" ( |
| <thead> | ||
| <tr> | ||
| <th>{{ T "fipValidity.issuer" }}</th> | ||
| <th>{{ T "trainCategory.additionalInformation" }}</th> |
There was a problem hiding this comment.
mhm it isn’t just "additional information" but the information itself :D
what do you think of "validity" or "acceptance"?
| {{- $statusIcon := index $iconMapping $status -}} | ||
| <tr> | ||
| <td> | ||
| {{ partial "link" (dict "Destination" (print "/operator/" .File.ContentBaseName) "Text" .Title "Page" $.page) }} |
There was a problem hiding this comment.
do we need the link here? The usecase fot the dialog is to look up the rules for my own operator and i don’t need the fip information about my own operator, do i?

This FIP validity can now be tracked per issuer. As example, add the following Params to the operator page operator1:
This means that employees of operator2 have 4 fields FIP coupopns, relatives don't get any coupons and there is 50% discount on FIP reduced tickets.
Resolves #446
TODO:
__info-buttonwith internal button