Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Handle null Telephone Numbers (#579) #767

wants to merge 1 commit into from

Conversation

T2brozz
Copy link

@T2brozz T2brozz commented Oct 26, 2020

Description
Changed return value from '' to No phone number
Issue
Closes #579

Changed return value from '' to No phone number
@T2brozz T2brozz changed the title Changed return value (#579) Fixed Handle null Telephone Numbers (#579) Oct 26, 2020
@T2brozz T2brozz changed the title Fixed Handle null Telephone Numbers (#579) fix: Handle null Telephone Numbers (#579) Oct 26, 2020
@github-actions
Copy link

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.

@mschwrdtnr
Copy link
Member

/azp run T-Systems-MMS.phonebook-preview

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mschwrdtnr
Copy link
Member

Hey @T2brozz, thanks for your first contribution 🎉

We will check the code and take a look at the demo :)

@mschwrdtnr
Copy link
Member

@DanielHabenicht preview still not working?

@DanielHabenicht
Copy link
Collaborator

DanielHabenicht commented Oct 28, 2020

@DanielHabenicht preview still not working?

nope, again. Seems like azure Kubernetes pricing has increased leading to the cluster not being up anymore.

@T2brozz
Copy link
Author

T2brozz commented Oct 28, 2020

Wait. Did I made a misstake?

@DanielHabenicht
Copy link
Collaborator

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, '');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

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.

Copy link
Collaborator

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?

Copy link
Author

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"

Copy link
Member

@mschwrdtnr mschwrdtnr Oct 30, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle null Telephone Numbers instead of displaying empty string
3 participants