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

Mention users with dash #2258

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Conversation

1-alex98
Copy link
Member

@1-alex98 1-alex98 commented Jun 1, 2021

Fixes #2202

@1-alex98 1-alex98 requested a review from Askaholic June 1, 2021 20:49
@1-alex98
Copy link
Member Author

1-alex98 commented Jun 1, 2021

Tried testing it is impossible

@codecov
Copy link

codecov bot commented Jun 1, 2021

Codecov Report

Merging #2258 (d7a1b0a) into develop (c982bba) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##             develop    #2258      +/-   ##
=============================================
+ Coverage      56.55%   56.57%   +0.02%     
- Complexity      3771     3773       +2     
=============================================
  Files            520      520              
  Lines          19712    19712              
  Branches        1171     1171              
=============================================
+ Hits           11149    11153       +4     
+ Misses          7946     7943       -3     
+ Partials         617      616       -1     
Impacted Files Coverage Δ
...forever/client/chat/AbstractChatTabController.java 80.92% <100.00%> (+1.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c982bba...d7a1b0a. Read the comment docs.

@Askaholic
Copy link
Collaborator

Can’t you write some tests like I did on the issue. Make a Matcher and call find.

What about users who’s name starts with a dash?

@1-alex98
Copy link
Member Author

1-alex98 commented Jun 3, 2021

Well the code does a shit tone of other things including reading files etc. and because we do not load a spring context this is incredible hard to mock away. Well I could just refactor to make it more testable I guess

@1-alex98
Copy link
Member Author

1-alex98 commented Jun 4, 2021

I stay with can not be tested

@Askaholic
Copy link
Collaborator

Just test the regex. You don’t need to test the code that uses the regex.

Just do something like

assertTrue(CHAT_PATTERN.matcher("Hello Box-!").find();

Whatever the syntax is

@Sheikah45 Sheikah45 merged commit 426b5a8 into develop Jun 8, 2021
@Sheikah45 Sheikah45 deleted the bugfix/#2202-user-with-dash-mention branch June 8, 2021 10:58
@@ -250,7 +250,7 @@ public void setReceiver(String receiver) {
}

public void initialize() {
mentionPattern = Pattern.compile("\\b(" + Pattern.quote(userService.getUsername()) + ")\\b", CASE_INSENSITIVE);
mentionPattern = Pattern.compile("\\b(" + Pattern.quote(userService.getUsername()) + ")\\b|[ ]", CASE_INSENSITIVE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Btw this doesn’t catch the case where the username is the only thing on the line. Like, this will not match the string "Box-".

Sheikah45 added a commit that referenced this pull request Jun 12, 2021
Sheikah45 added a commit that referenced this pull request Jun 12, 2021
mrchris2000 pushed a commit to mrchris2000/downlords-faf-client that referenced this pull request Apr 15, 2022
mrchris2000 pushed a commit to mrchris2000/downlords-faf-client that referenced this pull request Apr 15, 2022
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.

mentionPattern does not match names that end in -
3 participants