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

Fix(#87): Add Some Basic Validations For Better UX #89

Merged
merged 15 commits into from
Jul 13, 2022
Merged

Fix(#87): Add Some Basic Validations For Better UX #89

merged 15 commits into from
Jul 13, 2022

Conversation

ozair-dev
Copy link
Contributor

@ozair-dev ozair-dev commented Jul 4, 2022

  • Max length limit for Tag Name while creation: 25 Characters

  • All Tag Names Should Be In English, So Translate it or don't allow it

  • Users Per Page Should Be 15, not 16

  • Validate the language users are inputting, dont allow any abuses or curses

I have added a function censorBadWords (which can take string, object or array of strings as arguments and convert the bad words to censored words) to the services directory and it is being used in the formSubmit before dispatching.

Screenshot from 2022-07-05 03-57-14
Screenshot from 2022-07-05 03-54-34

Please review the PR and let me know if you want more changes.

@vercel
Copy link

vercel bot commented Jul 4, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @Mayank0255 on Vercel.

@Mayank0255 first needs to authorize it.

@ozair-dev ozair-dev changed the title Dev Fix(#87): Add Some Basic Validations For Better UX Jul 5, 2022
@vercel
Copy link

vercel bot commented Jul 5, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
stackoverflow-clone-frontend ✅ Ready (Inspect) Visit Preview Jul 13, 2022 at 8:08AM (UTC)

@Mayank0255 Mayank0255 added the enhancement Improvement in an existing feature or expanding a feature label Jul 5, 2022
@Mayank0255 Mayank0255 linked an issue Jul 5, 2022 that may be closed by this pull request
@ozair-dev
Copy link
Contributor Author

The censorship is applied when a new post, comment or answer is submitted. Please try adding a new post and test for yourself if it works.
And if you don't like it this way! then I'll apply censorship to the post, answer, comment preview instead of submission stage

@Mayank0255
Copy link
Owner

@ozair-dev
I think you can remove censorchip from submission, as I think we can store uncensored data, and display with censorship

@ozair-dev
Copy link
Contributor Author

@Mayank0255 sure,
I will censor comments, answers, and the posts. But I think the tags should not be censored because what's the point of showing a tag named s**t? Instead, I won't allow swear words for tags while submission. By doing that, we wont get new abusive tags.

@ozair-dev
Copy link
Contributor Author

@Mayank0255
please review the PR,
I'hv censored the bad words in a way that only first and last letter is shown.
Also note that some abusive words won't be censored when joined with other words, for example ass in assume wont be censored, like wise pooppoopoop wont be censored. This is just how the package (which I used for censoring) works. But you'll see that it works very well in common scenerios.
Screenshot from 2022-07-07 01-20-59

Screenshot from 2022-07-07 01-27-08

Tags are not censored. Instead, they are detected at post submission and an error is shown
Screenshot from 2022-07-07 01-33-44

@Mayank0255
Copy link
Owner

@ozair-dev cool,
One suggestion, can change the tags censor text to, "Inappropriate words are not allowed"

As the current text seems like that only tags cannot be Inappropriate 😅, though they can write inappropriate words for title and body but why tell them that

@ozair-dev
Copy link
Contributor Author

One suggestion, can change the tags censor text to, "Inappropriate words are not allowed"

Can do, but

As the current text seems like that only tags cannot be Inappropriate sweat_smile, though they can write inappropriate words for title and body but why tell them that

If the title or body contain inappropriate words, it won't terminate the dispatching of the formData, so user is actually allowed to write inappropriate words there, but If the tags contain the inappropriate words then dispatching is stopped, so I specifically mentioned tags cannot contains inappropriate words.
But I'll still make the changes as you have asked for.

@ozair-dev
Copy link
Contributor Author

@Mayank0255 Just a quick heads-up before you merge the changes!
While matching and replacing bad words, a bunch a regex operations happens behind the scenes on each post, comment and answer, and this makes the page loading very slow. And in my opinion, to avoid this, the data must be censored on each submission. So we don't have to use regex on page load.
I don't know how big companies do these censorship on data, but as far as I know, we just can't (or shouldn't) do it after data retrieval.

@Mayank0255
Copy link
Owner

By how much is the page loading time increasing?

@ozair-dev
Copy link
Contributor Author

On my machine, it's taking about 43 seconds.
IMO, each post takes anywhere from 100-300ms, (speed may vary depending on processor speed), so it is wise to censor bad words on submission.

Copy link
Owner

@Mayank0255 Mayank0255 left a comment

Choose a reason for hiding this comment

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

@ozair-dev

Pls look into this and then see that whats the difference between the response time

export const getAnswers = (id) => async (dispatch) => {
try {
const res = await axios.get(config.BASE_URL + `/api/posts/answers/${id}`);

dispatch({
type: GET_ANSWERS,
payload: res.data.data,
payload: res.data.data.map(({body, ...rest})=>({...censorBadWords({body}), ...rest})),
Copy link
Owner

Choose a reason for hiding this comment

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

Here we are looping on the complete response and then changing the text, and then while rendering we are anyways looping through the result.

The better approach would be to convert the text while rending so that we can avoid an extra loop, and avoid a delete and an insert operation

export const getComments = (id) => async (dispatch) => {
try {
const res = await axios.get(config.BASE_URL + `/api/posts/comments/${id}`);

dispatch({
type: GET_COMMENTS,
payload: res.data.data,
payload: res.data.data.map(({body, ...rest})=>({...censorBadWords({body}), ...rest})),
Copy link
Owner

Choose a reason for hiding this comment

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

Same can be done here

// Get posts
export const getPosts = () => async (dispatch) => {
try {
const res = await axios.get(config.BASE_URL + '/api/posts');

dispatch({
type: GET_POSTS,
payload: res.data.data,
payload: res.data.data.map(({title, body, ...rest})=>({...censorBadWords({title, body}), ...rest})),
Copy link
Owner

Choose a reason for hiding this comment

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

Over here there's no loop, but to remain consistency and to avoid insert and delete operation we can just convert the text while rendering

@ozair-dev
Copy link
Contributor Author

I'm busy these couple of days. Will try when I am free In sha'Allah.

@Mayank0255
Copy link
Owner

I'm busy these couple of days. Will try when I am free In sha'Allah.

Sure, no problem

@ozair-dev
Copy link
Contributor Author

I have removed censoring for posts on data retrieval and instead, I am censoring title and body of each PostItem.compoent component on render. It stills takes about 43 seconds for HomePage to load. And additional 43 seconds (more or less depending on amount of text and machine's speed etc) on each pagination.

Here in the screenshot, you can see in console how much time each text is taking to be censored. Even the amount of time taken to censor even a small string is not negligible

Long story short, censoring text on rendering also does not reduce the loading time.

Screenshot from 2022-07-12 15-53-12

Deploy the changes and see the time time taken to censor each text in console to get a better idea.

@Mayank0255
Copy link
Owner

@ozair-dev

questions page is breaking

@ozair-dev
Copy link
Contributor Author

fixed

@Mayank0255
Copy link
Owner

@ozair-dev looks good enough, just remove the logs and I will merge

@ozair-dev
Copy link
Contributor Author

Done

@Mayank0255 Mayank0255 merged commit 64a590b into Mayank0255:master Jul 13, 2022
@Mayank0255
Copy link
Owner

@ozair-dev
Merged🎉🎉

@ozair-dev ozair-dev deleted the dev branch July 13, 2022 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement in an existing feature or expanding a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Frontend]: Add Some Basic Validations For Better UX
2 participants