Skip to content

added check for comment length #537

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prakharchoudhary
Copy link

@prakharchoudhary prakharchoudhary commented Jun 30, 2017

Fixes #533

Comment length should be greater than 15.
Please review this PR.
@kracekumar @RishabhJain2018

@coveralls
Copy link

coveralls commented Jun 30, 2017

Coverage Status

Coverage increased (+0.05%) to 67.423% when pulling c07e8a1 on prakharchoudhary:min_words into 96e0aed on pythonindia:master.

@@ -22,7 +22,7 @@ def test_reviewer_private_comment(self, settings, login, conferences,
data = {'comment': 'Test', 'private': True}

response = client.post(url, data)

print(response.url)
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the print from here.

"django.core.context_processors.debug",
)

EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'

INSTALLED_APPS += ('django_extensions',)
INSTALLED_APPS += ('django_extensions',) # noqa
Copy link
Member

Choose a reason for hiding this comment

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

To fix this, you need to follow PEP8 guideline, which says there should be a space after comma in a tuple. You can remove the #noqa and fix the actual issue.

}
}

TEMPLATE_CONTEXT_PROCESSORS += (
TEMPLATE_CONTEXT_PROCESSORS += ( # noqa
Copy link
Member

Choose a reason for hiding this comment

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

I don't it requires a # noqa here, can you provide the actual error message you are getting.

@@ -11,16 +11,16 @@
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.sqlite3',
'NAME': os.path.join(ROOT_DIR, 'test.sqlite3'),
'NAME': os.path.join(ROOT_DIR, 'test.sqlite3'), # noqa
Copy link
Member

Choose a reason for hiding this comment

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

I don't it requires a # noqa here, can you provide the actual error message you are getting.

@@ -103,7 +103,7 @@
"junction.base.context_processors.site_info",
],

'debug': DEBUG,
'debug': DEBUG, # noqa
Copy link
Member

Choose a reason for hiding this comment

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

I don't it requires a # noqa here, can you provide the actual error message you are getting.

$("#comment").click(function() {
$(".alert").remove()
var text = $("textarea").val();
if (text.length >= 15) {
Copy link
Member

Choose a reason for hiding this comment

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

Please make 15 as a variable and this variable needs to configurable via the Django settings and must not be hard-coded into template.

@palnabarun
Copy link
Member

@prakharchoudhary Are you still working on this?

@prakharchoudhary
Copy link
Author

@prakharchoudhary Are you still working on this?

No, I am not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have minimum words in the public comment
5 participants