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

address.secondaryAddress doesn't use faker.definitions.address.secondary_address #676

Closed
xDivisionByZerox opened this issue Mar 26, 2022 · 3 comments · Fixed by #680
Closed
Assignees
Labels
c: bug Something isn't working p: 1-normal Nothing urgent

Comments

@xDivisionByZerox
Copy link
Member

Clear and concise description of the problem

The current implementation of faker.address.secondaryAddress uses a hardcoded string array to choose from instead of the definition one. This means locales are ignored for this method.

faker/src/address.ts

Lines 311 to 316 in e62fd15

secondaryAddress(): string {
return this.Helpers.replaceSymbolWithNumber(
// TODO ST-DDT 2022-01-30: this.faker.definitions.address.secondary_address
this.faker.random.arrayElement(['Apt. ###', 'Suite ###'])
);
}

Suggested solution

Use faker.definitions.address.secondary_address instead of the hardcoded string array

secondaryAddress(): string {
  return this.Helpers.replaceSymbolWithNumber(
    this.faker.random.arrayElement(this.faker.definitions.address.secondary_address)
  );
}

Alternative

No response

Additional context

No response

@Shinigami92
Copy link
Member

Is this related to #88?

@xDivisionByZerox
Copy link
Member Author

I would say yes. But it breaks down the issue described in #88 in a smaller task that can be picked up.

@Shinigami92 Shinigami92 added good first issue Good for newcomers p: 1-normal Nothing urgent c: locale Permutes locale definitions and removed s: pending triage Pending Triage labels Mar 26, 2022
@Shinigami92 Shinigami92 moved this to Todo in Faker Roadmap Mar 26, 2022
@Shinigami92 Shinigami92 added this to the v6.2 - New small features milestone Mar 26, 2022
@xDivisionByZerox
Copy link
Member Author

I provided #680 to fix this.

@Shinigami92 Shinigami92 linked a pull request Mar 26, 2022 that will close this issue
@Shinigami92 Shinigami92 removed the good first issue Good for newcomers label Mar 26, 2022
@Shinigami92 Shinigami92 moved this from Todo to Awaiting Review in Faker Roadmap Mar 26, 2022
@Shinigami92 Shinigami92 added c: bug Something isn't working and removed c: locale Permutes locale definitions labels Mar 26, 2022
Repository owner moved this from Awaiting Review to Done in Faker Roadmap Mar 26, 2022
@ST-DDT ST-DDT removed this from Faker Roadmap Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants