-
-
Notifications
You must be signed in to change notification settings - Fork 804
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
Add warning for the length of the group name #2122
Merged
+65
−30
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
2e8c005
add warning of length of group
IronJam11 a58f8d9
fix minor issues to pass the tests
IronJam11 19879b2
add regression test
IronJam11 1bbd2d9
update the structure of the test code
IronJam11 cc46101
add the erased test
IronJam11 50f53bc
remove unnecessary assertions
IronJam11 48533bd
inline the error_message
IronJam11 a13f9eb
change the function name to require_valid_group_name and remove asser…
IronJam11 792c79e
applying the same changes for channel
IronJam11 ae711a3
refactor all to use for loop
IronJam11 027aad1
remove lint errors
IronJam11 8b6c547
correct the return instance
IronJam11 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 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 |
---|---|---|
|
@@ -5,7 +5,8 @@ | |
import string | ||
import time | ||
from copy import deepcopy | ||
|
||
import logging | ||
logger = logging.getLogger(__name__) | ||
from django.conf import settings | ||
from django.core.signals import setting_changed | ||
from django.utils.module_loading import import_string | ||
|
@@ -160,7 +161,10 @@ def valid_channel_name(self, name, receive=False): | |
raise TypeError(self.invalid_name_error.format("Channel", name)) | ||
|
||
def valid_group_name(self, name): | ||
if self.match_type_and_length(name): | ||
logger.debug(f"Validating group name: {name}, Length: {len(name)}") # Log group name length | ||
if isinstance(name, str): | ||
if len(name) >= self.MAX_NAME_LENGTH: | ||
raise False | ||
if bool(self.group_name_regex.match(name)): | ||
return True | ||
raise TypeError(self.invalid_name_error.format("Group", name)) | ||
|
@@ -201,6 +205,7 @@ async def flush(self): | |
raise NotImplementedError("flush() not implemented (flush extension)") | ||
|
||
async def group_add(self, group, channel): | ||
print("Hello") | ||
raise NotImplementedError("group_add() not implemented (groups extension)") | ||
|
||
async def group_discard(self, group, channel): | ||
|
@@ -336,19 +341,28 @@ def _remove_from_groups(self, channel): | |
|
||
# Groups extension | ||
|
||
|
||
async def group_add(self, group, channel): | ||
""" | ||
Adds the channel name to a group. | ||
""" | ||
# Check the inputs | ||
assert self.valid_group_name(group), "Group name not valid" | ||
print(f"Validating group name: {group}") | ||
assert self.valid_group_name(group), f"Group name must be less than {self.MAX_NAME_LENGTH} characters." | ||
assert self.valid_channel_name(channel), "Channel name not valid" | ||
# Add to group dict | ||
|
||
# Check the length of the group name | ||
if len(group) >= self.MAX_NAME_LENGTH: | ||
raise TypeError(f"Group name must be less than {self.MAX_NAME_LENGTH} characters, but got {len(group)}.") | ||
|
||
# Add to group dict | ||
self.groups.setdefault(group, {}) | ||
self.groups[group][channel] = time.time() | ||
|
||
|
||
async def group_discard(self, group, channel): | ||
# Both should be text and valid | ||
print(f"Discarding channel {channel} from group {group}") # Log group name | ||
assert self.valid_channel_name(channel), "Invalid channel name" | ||
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. Same here as aboev |
||
assert self.valid_group_name(group), "Invalid group name" | ||
# Remove from group set | ||
|
Oops, something went wrong.
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.
This (and others) need removing.