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 37
Order form is dropping second last names unexpectedly #3113
Merged
+186
−2
Merged
Changes from 1 commit
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
Add test to verify that proper first name is returned given customer …
…name
commit 0266ff201d82619cdbd2525287dbffde61fb2594
There are no files selected for viewing
This file contains 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,22 @@ class WCCustomerMapperTest { | |
// THEN | ||
assertThat(result.isPayingCustomer).isEqualTo(false) | ||
} | ||
|
||
@Test | ||
@Suppress("LongMethod") | ||
fun `given customer name, then first name is properly assigned`() { | ||
// given | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. np: the comments are upper case in the other tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done: 8f338d2 |
||
val siteId = 23 | ||
val site = SiteModel().apply { id = siteId } | ||
|
||
val customerDTO = CustomerFromAnalyticsDTO(name = "firstname with a very long last name") | ||
|
||
// when | ||
val result = mapper.mapToModel(site, customerDTO) | ||
|
||
// then | ||
with(result) { | ||
assertEquals("firstname", firstName) | ||
} | ||
} | ||
} |
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.