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

Adds heartbeat ajax functions from gf, Fixes enqueue issue and Fixes … #2242

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

omarkasem
Copy link
Collaborator

@omarkasem omarkasem commented Dec 19, 2024

  • Implements Add locking to entries when they are being edited #950
  • Adds heartbeat ajax functions from gf GFLocking class
  • Fixes enqueue issue that had a condition of joined forms in one view which i don't think can happen anymore(Loading multiple forms in one view)
  • Fixes Modal issue in frontend where gf locking script depends on #wpwrap which exists in dashboard
  • Fixes big issue in check_user_cap_edit_entry where it's called using heartbeat ajax functions and the issue is we can't pass $view_id to make sure the user_edit option exist as it happens in gf locking script, So i fixed it in a way that won't affect any other requests other than ajax requests

💾 Build file (ad1088d).

@omarkasem omarkasem linked an issue Dec 19, 2024 that may be closed by this pull request
@omarkasem omarkasem self-assigned this Dec 19, 2024
@omarkasem omarkasem requested a review from mrcasual December 19, 2024 17:24
@mrcasual
Copy link
Collaborator

@Mwalek, please test.

@Mwalek
Copy link

Mwalek commented Dec 20, 2024

@mrcasual @omarkasem I noticed the following:

  1. Edit locking does not work when editing in a lightbox.
  2. When control is requested by a user with the subscriber role, the user is shown their own gravatar and not the gravatar of the user that's currently editing.
  3. Potential Enhancement: When control is requested, upon the person editing accepting the request, the user that requested access must click the "take control" button. This takes the user to the entry URL, instead of the edit entry URL.

Summary

  • Edit locking in lightbox
  • Wrong gravatar shown
  • Enhancement to go directly to edit URL

@omarkasem
Copy link
Collaborator Author

@Mwalek Both issues are solved.
About the lightbox part, I tried it but it will very hard to do as the whole gravity forms locking depends on getting the entry id being inside the edit entry page and we will also have to load the whole locking js scripts inside the entries view list, Let me know if it's really important and i can spend some time on it

@Mwalek
Copy link

Mwalek commented Jan 2, 2025

@omarkasem thank you! I confirmed that the 2 issues are solved. The lightbox issue should not necessarily be fixed. I didn't face any other issues during tests 👌

cc. @mrcasual

}

protected function check_lock_request( $object_id ) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

@omarkasem, please do not add line breaks. We keep repeating asking this in almost every PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mrcasual Thanks for the reminder but I copied the functions from the gf-locking class like the rest of the file was, I was just trying to stay consistent with the styling in the same file


public function heartbeat_refresh_lock( $response, $data, $screen_id ) {
$heartbeat_key = 'gform-refresh-lock-entry';
if ( array_key_exists( $heartbeat_key, $data ) ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@omarkasem, please exit early.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mrcasual 90% of our class is copied from the gf-locking class as we depend on it's functions in everything in this locking feature, Should we change their functions or keep it as it is?

Copy link
Collaborator

@mrcasual mrcasual Jan 13, 2025

Choose a reason for hiding this comment

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

@omarkasem, that's fine, thanks for the context. Perhaps adding a @since and @see tags would help explain things for someone lacking such context.


public function heartbeat_request_lock( $response, $data, $screen_id ) {
$heartbeat_key = 'gform-request-lock-entry';
if ( array_key_exists( $heartbeat_key, $data ) ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@omarkasem, please exit early.


$object_id = $received['objectID'];

if ( ( $user_id = $this->check_lock( $object_id ) ) && ( $user = get_userdata( $user_id ) ) ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@omarkasem, please avoid using inline assignments as they make code harder to read.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mrcasual Thanks for the note and I totally agree with the point, It's just copied as mentioned before and I tried to not change anything so we stay consistent with the gf locking class

@rafaehlers
Copy link
Contributor

@Mwalek Both issues are solved. About the lightbox part, I tried it but it will very hard to do as the whole gravity forms locking depends on getting the entry id being inside the edit entry page and we will also have to load the whole locking js scripts inside the entries view list, Let me know if it's really important and i can spend some time on it

@mrcasual the only thing missing in this PR is supporting the edit lock functionality inside a lightbox. Can you help Omar with the steps necessary?

@mrcasual
Copy link
Collaborator

@Mwalek Both issues are solved. About the lightbox part, I tried it but it will very hard to do as the whole gravity forms locking depends on getting the entry id being inside the edit entry page and we will also have to load the whole locking js scripts inside the entries view list, Let me know if it's really important and i can spend some time on it

@mrcasual the only thing missing in this PR is supporting the edit lock functionality inside a lightbox. Can you help Omar with the steps necessary?

@rafaehlers, is this a must-have feature?

@rafaehlers
Copy link
Contributor

@Mwalek Both issues are solved. About the lightbox part, I tried it but it will very hard to do as the whole gravity forms locking depends on getting the entry id being inside the edit entry page and we will also have to load the whole locking js scripts inside the entries view list, Let me know if it's really important and i can spend some time on it

@mrcasual the only thing missing in this PR is supporting the edit lock functionality inside a lightbox. Can you help Omar with the steps necessary?

@rafaehlers, is this a must-have feature?

Well, I think it is a must-have because the Edit Locking option doesn't specify where this happens or not, so they'll assume it's everywhere:

image

Let's not get lazy here.

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.

Add locking to entries when they are being edited
4 participants