Comments

patelmayank7552 created an issue. See original summary.

patelmayank7552’s picture

Assigned: Unassigned » patelmayank7552
purvitagupta’s picture

StatusFileSize
new4.07 KB

Hi, here is the updated patch for coding standard.

purvitagupta’s picture

Status: Active » Needs review
raghavendra a m’s picture

Priority: Normal » Minor
Issue tags: +IttHackathon
StatusFileSize
new75.93 KB

Hi purvitagupta, I review the contact_poup_patch.patch file. It's working fine

raghavendra a m’s picture

Status: Needs review » Reviewed & tested by the community

#5 status

avpaderno’s picture

Version: 8.x-1.1 » 8.x-1.x-dev
Status: Reviewed & tested by the community » Needs work
+      // Deny access the contact form link if we are not on a user-related page.

It's Deny access to the contact form link. (Without to is ungrammatical.)

// Don't display the link if the user is on profile page.

It's on the profile page, but that means any profile page. If the users can see the link if they are on their profile page, it should be Don't display the link if the users are on their profile page.

purvitagupta’s picture

StatusFileSize
new4.05 KB
purvitagupta’s picture

Status: Needs work » Needs review
avpaderno’s picture

Title: Coding Standard Issue » Coding standard issues
Status: Needs review » Needs work
-      // Deny access to the contact form link if we are not on a user related page
+      // Don't display the link if the user is on profile page.

It doesn't display the link, but it gives permission to users to see the the link. See also my previous comment.

-      // Do not display the link if the user is on his profile page.
+      // Don't display the link if the user is on profile page.

See my previous comment.

Also, is it necessary to change code comments when the change doesn't fix coding issues?

Deepthi kumari’s picture

Status: Needs work » Needs review
StatusFileSize
new4.09 KB

Attached the patch. Please review.

avpaderno’s picture

Status: Needs review » Needs work
-      // Deny access to the contact form link if we are not on a user related page
-      // or we have no access to that page.
+      // Deny access to the contact form link
+      // if we are not on a user related page.

Lines should not be longer than 80 character, but this doesn't mean starting a new line after 40 characters.

     // Access to other contact forms is equal to the permission of the
-    // entity.contact_form.canonical route. Once https://www.drupal.org/node/2724503
+    // entity.contact_form.canonical route.
+    // Once https://www.drupal.org/node/2724503
     // has landed, see if we can support per form access permission.

Assuming the old line was too long, starting a new line is fine, but Once https://www.drupal.org/node/2724503 doesn't seem to get closer to 80 characters.

The rest of the patch is fine.

Deepthi kumari’s picture

StatusFileSize
new4.16 KB

Updated the patch.

Deepthi kumari’s picture

Status: Needs work » Needs review
avpaderno’s picture

Status: Needs review » Reviewed & tested by the community

I think it's good to go.

rakesh.gectcr’s picture

rakesh.gectcr’s picture

Thank you all for your contribution.

rakesh.gectcr’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.