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

User blocker #201

Merged
merged 2 commits into from
May 7, 2021
Merged

User blocker #201

merged 2 commits into from
May 7, 2021

Conversation

Radi85
Copy link
Owner

@Radi85 Radi85 commented May 3, 2021

closes #143

  • This will add the required models/managers and utils to allow comment's moderator to block users or emails of commenting or reaction with the comments.
  • The view implementation along with the API are implemented in the next commit.

closes #190

  • This will use status code 403 for access denied in comment mixins and handle it in JS functions.
  • Implement blocking views and the API.

@abhiabhi94
Copy link
Collaborator

can the two features be in separate pull requests? it could potentially make the diff lesser and easier to view on github.

@Radi85
Copy link
Owner Author

Radi85 commented May 3, 2021

can the two features be in separate pull requests? it could potentially make the diff lesser and easier to view on github.

You can see that there are views implementation in the second commit while in fact it should belong to the first one. It was pain to seperate them.

comment/conf/defaults.py Outdated Show resolved Hide resolved
comment/messages.py Outdated Show resolved Hide resolved
comment/messages.py Outdated Show resolved Hide resolved
@abhiabhi94
Copy link
Collaborator

I will try to review more when I have some more time today maybe, I have only viewed close to 15 files only 😩 as of now, which also include the rst one 😆 that i just skipped.

but from what i could see, this think you haven't made the change to pass an extra variable from the templates to check whether DEBUG is true, and show the error on the frontend for easier reporting/debugging when javascript fails.

pretty similar hack to the one that we use for gettext here.

{% if is_translation_allowed %}
<script type="text/javascript" src="{% url 'comment:javascript-catalog' %}"></script>
{% else %}
<script>
/* This will allow django to detect strings for javascript translations */
const gettext = str => str;
</script>
{% endif %}

@Radi85
Copy link
Owner Author

Radi85 commented May 3, 2021

@abhiabhi94,
BTW, away from work. How are you doing over there with the current situation. I hope you and your family are healthy and safe.

@abhiabhi94
Copy link
Collaborator

abhiabhi94 commented May 3, 2021

BTW, away from work. How are you doing over there with the current situation. I hope you and your family are healthy and safe.

The situation is very bad in India, as you might be aware from news. Some of my family members had tested positive for coronavirus in the past month, but they are stable now. I had a freelancing/part-time job as a python instructor at a place, which is also shut as of now.

The most scary thing is the death numbers. The actual numbers could be more than 10 times in most states than the ones you see in news. The governments are hiding actual numbers, but it is sort of an open secret here. Each day, we get to hear about new deaths of nearby people or someone that we might have known.

That said, how are you and your family there? What's the situation like there?

@abhiabhi94
Copy link
Collaborator

As far as i can see, i think the same permission group used for moderation is also used for blocking. in this case, we will probably also have to edit the signals package to make the permissions groups(in the database) when either of those settings is used.

comment/mixins.py Outdated Show resolved Hide resolved
comment/utils.py Outdated Show resolved Hide resolved
comment/views/blocker.py Outdated Show resolved Hide resolved
@Radi85
Copy link
Owner Author

Radi85 commented May 4, 2021

The situation is very bad in India, as you might be aware from news. Some of my family members had tested positive for coronavirus in the past month, but they are stable now. I had a freelancing/part-time job as a python instructor at a place, which is also shut as of now.

I hope this pandemic situation will be under control soon. Stay safe and healthy!

That said, how are you and your family there? What's the situation like there?

We are fine and the situation seems to be under control here since the country is still locked down.

@Radi85
Copy link
Owner Author

Radi85 commented May 4, 2021

but from what i could see, this think you haven't made the change to pass an extra variable from the templates to check whether DEBUG is true, and show the error on the frontend for easier reporting/debugging when javascript fails.

I am not sure fully understood but I added error log in JS file whenever an error thrown.

@abhiabhi94
Copy link
Collaborator

I am not sure fully understood but I added error log in JS file whenever an error thrown.

so, now the error will directly be shown in the console. What i thought would be better to show these only when in developoment i.e. DEBUG = True inside settings. For that we will need to define a variable debug or some other name from the template when the project is being run in development mode, and only log errors when that variable is defined.

@Radi85
Copy link
Owner Author

Radi85 commented May 6, 2021

@abhiabhi94,
Shall we merge this?

comment/messages.py Outdated Show resolved Hide resolved
comment/tests/base.py Outdated Show resolved Hide resolved
comment/views/blocker.py Outdated Show resolved Hide resolved
docs/source/settings.rst Outdated Show resolved Hide resolved
Radi85 added 2 commits May 7, 2021 10:33
This will add the required models/managers and utils to allow comment's
moderator to block users or emails of commenting or reaction with the
comments.
The view implementation along with the API are implemented in the next
commit.
This will use status code 403 for access deneid in comment mixins and handle it in
JS functions.
Implement blocking views and the API.
Copy link
Collaborator

@abhiabhi94 abhiabhi94 left a comment

Choose a reason for hiding this comment

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

Promise me not to make so much of changes in one pull request unless it is absolutely necessary 😄 .

@Radi85
Copy link
Owner Author

Radi85 commented May 7, 2021

Promise me not to make so much of changes in one pull request unless it is absolutely necessary 😄 .

I am sorry, couldn't really split it. BTW the API list view is not paginated and we need to open an issue for it.

@Radi85 Radi85 merged commit 877d4e6 into develop May 7, 2021
@abhiabhi94
Copy link
Collaborator

I am sorry, couldn't really split it. BTW the API list view is not paginated and we need to open an issue for it.

Maybe we can keep the issue open to see if there are requirements for it to the people. If we have positive feedback, it can be implemented.

@Radi85 Radi85 deleted the user-blocker branch May 7, 2021 09:58
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.

Handle access permissions Block user or email from posting and reacting with comments
2 participants