-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Handle null Telephone Numbers (#579) #767
base: master
Are you sure you want to change the base?
Conversation
Changed return value from '' to No phone number
We are following Conventional Commit Format. Please make sure that your Pull Request title does conform to this format, otherwise it might not be released. |
/azp run T-Systems-MMS.phonebook-preview |
Azure Pipelines successfully started running 1 pipeline(s). |
Hey @T2brozz, thanks for your first contribution 🎉 We will check the code and take a look at the demo :) |
@DanielHabenicht preview still not working? |
nope, again. Seems like azure Kubernetes pricing has increased leading to the cluster not being up anymore. |
Wait. Did I made a misstake? |
no, but normally we could see what you did in our demo environment which is populated for each Pull Request. Unfortunately, that's not possible because it is getting to expensive. So we will have to preview it locally. I will have a look later. |
@@ -6,6 +6,6 @@ import { Pipe, PipeTransform } from '@angular/core'; | |||
export class TelephonePipe implements PipeTransform { | |||
// TODO: #579 | |||
public transform(telephoneNumber: null | string): string { | |||
return telephoneNumber == null ? '' : telephoneNumber.replace(/[^+0-9]/g, ''); | |||
return telephoneNumber == null ? 'No phone number' : telephoneNumber.replace(/[^+0-9]/g, ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the translation service as described here: https://t-systems-mms.github.io/phonebook/pages/development-guides/frontend/translation-guide.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that german users will get the message in german.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I am still thinking about a more general message that we could also reuse for other fields (e.g. Team assistant), maybe "not know" @mschwrdtnr @paule96 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of you want to use a general message. I would suggest "Not available" and in german "nicht verfügbar"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A more generally one could be just "unknown". This will probably make more sense for other fields like your example @DanielHabenicht
Description
Changed return value from '' to No phone number
Issue
Closes #579