This repository was archived by the owner on Feb 4, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 38
Order form is dropping second last names unexpectedly #3113
Merged
Merged
Changes from 10 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
e885fec
Refactor first name and last name detection of customer logic
AnirudhBhat 0266ff2
Add test to verify that proper first name is returned given customer …
AnirudhBhat 8f355b0
Remove unnecessary annotation
AnirudhBhat 6e3f63b
add test to verify proper last name is detected
AnirudhBhat 2b2a46d
add test to verify empty string is returned when customer name is null
AnirudhBhat 6bc6314
add test to verify full string is returned when customer name has no …
AnirudhBhat 27f2158
add test to verify empty string is returned for last name when custom…
AnirudhBhat bba2b8d
add test to verify empty string is returned for last name when custom…
AnirudhBhat 4ca0bc6
add test to verify proper string is returned for first name when cust…
AnirudhBhat d76f381
add test to verify proper string is returned for last name when custo…
AnirudhBhat 3b5294e
Refactor logic to handle multiple spaces, leading and trailing spaces…
AnirudhBhat 2276918
Add test to verify leading space in customer name will still give us …
AnirudhBhat 1700a00
Add test to verify trailing space in customer name will still give us…
AnirudhBhat c4e267a
Add test to verify multiple in between space in customer name will st…
AnirudhBhat 2455c15
Add test to verify multiple in between space in customer name will st…
AnirudhBhat 8f338d2
Comment given, when, then in capitals to maintain consistency in file
AnirudhBhat e085d0c
Add comment
AnirudhBhat 7b6d579
Fix checkstyle error
AnirudhBhat 651e2b9
Fix checkstyle error
AnirudhBhat a243e08
Fix checkstyle error
AnirudhBhat 2aca001
Refactor first name and last name extraction logic
AnirudhBhat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,4 +273,124 @@ class WCCustomerMapperTest { | |
| // THEN | ||
| assertThat(result.isPayingCustomer).isEqualTo(false) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name, then first name is properly assigned`() { | ||
| // given | ||
|
||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = "firstname lastname") | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("firstname", result.firstName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name as null, then first name returns empty string`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = null) | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("", result.firstName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name with no space, then first name returns full string`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = "firstnamelastname") | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("firstnamelastname", result.firstName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name, then last name is properly assigned`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = "firstname lastname") | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("lastname", result.lastName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name as null, then last name returns empty string`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = null) | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("", result.lastName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name has no space, then last name returns empty string`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = "firstnamelastname") | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("", result.lastName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name has multiple spaces, then first name returns proper string`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = "firstname and a very long last name") | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("firstname", result.firstName) | ||
| } | ||
|
|
||
| @Test | ||
| fun `given customer name has multiple spaces, then last name returns proper string`() { | ||
| // given | ||
| val siteId = 23 | ||
| val site = SiteModel().apply { id = siteId } | ||
|
|
||
| val customerDTO = CustomerFromAnalyticsDTO(name = "firstname and a very long last name") | ||
|
|
||
| // when | ||
| val result = mapper.mapToModel(site, customerDTO) | ||
|
|
||
| // then | ||
| assertEquals("and a very long last name", result.lastName) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,7 @@ class WCCustomerMapper @Inject constructor() { | |
| } | ||
| } | ||
|
|
||
| private fun String?.firstNameFromName() = this?.split(" ")?.firstOrNull() ?: "" | ||
| private fun String?.lastNameFromName() = this?.split(" ")?.lastOrNull() ?: "" | ||
| private fun String?.firstNameFromName() = this?.substringBefore(" ") ?: "" | ||
|
|
||
| private fun String?.lastNameFromName() = this?.substringAfter(" ", "") ?: "" | ||
|
||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
np: in all tests, maybe assert both first and last names, because they kinda related and deducted from one string?
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.
I agree, my reasoning for splitting the test cases is mainly to understand why the test fails in future. There must be only one reason for the failure of tests IMHO. If we combine both, we need to see if the test failed because of logical error in the extraction of first name or last name which could be avoided if the test asserts just one thing.
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.
In my opinion, the one reason here - "if correct logic used in splitting the string". The current implementation, which consists of two private extensions, should not affect the test's logic. Currently, the test does not validate whether the splitter is functioning correctly; instead, it only checks that the first word is designated as the first name.