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

Inline edit triggers multiple saves, very slow #8886

Open
pgorod opened this issue Oct 6, 2020 · 1 comment
Open

Inline edit triggers multiple saves, very slow #8886

pgorod opened this issue Oct 6, 2020 · 1 comment
Labels
Area: Module Issues & PRs related to modules that do not have specific label Priority:Important Issues & PRs that are important; broken functions, errors - there are workarounds Status:Fix Proposed A issue that has a PR related to it that provides a possible resolution Type: Bug Bugs within the core SuiteCRM codebase

Comments

@pgorod
Copy link
Contributor

pgorod commented Oct 6, 2020

Issue

When using inline editing, there are two performance problems:

  1. It uses two Ajax calls to the server at the start, when one would suffice (this is not a bug, just an unfortunate design)
  2. It often fires more than one (sometimes a dozen!) Ajax calls saving the field, which is a slow operation. This is a bug! It's easy for a save operation to go over 10 seconds delay (which is unacceptable on a server with just one user and little data).

I've tracked it down to a call generated from a keypress event that fires more than once as long as the key is held down, even if just for a fraction of a second.

Expected Behavior

Inline edit should be quick and use just one Ajax call when starting, one when saving.

Possible Fix

In include/InlineEditing/inlineEditing.js, change this line

$(document).keypress(function(e) {

to

$(document).off('keypress').on('keypress', function(e) {

This fixes the bug.

Then, to improve part 1. of the issue mentioned above, refactor these two calls to be done in just one round-trip to the server, joining the two results in a single payload:

//Do ajax call to retrieve the validation for the field.
var validation = getValidationRules(field, module, id);
//Do ajax call to retrieve the html elements of the field.
var html = loadFieldHTML(field, module, id);

I might provide these features as a PR as soon as I see that PR's are getting merged normally again. Right now I don't see the point in just adding to the list.

Your Environment

  • SuiteCRM Version used: any recent version
@johnM2401 johnM2401 added Area: Module Issues & PRs related to modules that do not have specific label Priority:Important Issues & PRs that are important; broken functions, errors - there are workarounds Status:Fix Proposed A issue that has a PR related to it that provides a possible resolution Type: Bug Bugs within the core SuiteCRM codebase labels Oct 19, 2020
@pgorod
Copy link
Contributor Author

pgorod commented Oct 20, 2020

A bit of new information: there are actually 3 separate calls, not 2, when the field is a drop-down. So that can also be integrated in a single call.

I have this refactoring completed and I was able to to bring down the time to initiate an inline edit from 1.5 seconds to 0.5 seconds. That might seem a small difference, but it actually alleviates the server, feels a lot less sluggish to the user, and even prevents some user errors like retrying the double-click.

Also to elaborate on my reluctance to provide a PR just now. In my work on add-ons, I'm having serious trouble handling the problems of working from the custom dir instead of changing things in core; this compounds with the difficulties in managing the very different SuiteCRM versions that customers use.

If I make the PR, I will be effectively creating a new conflict with my own add-ons. I could try to manage this, if I thought the PR would go in swiftly, and that users would upgrade quickly. But if the merging gets delayed for ages, and the bugs deter users from upgrading for really long times, things just start getting too muddled.

That said, I do want to contribute this code, and will do so if asked; meanwhile I'll just wait for things to get back to normal here on GitHub (by normal I mean pre-2020 merging levels).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Module Issues & PRs related to modules that do not have specific label Priority:Important Issues & PRs that are important; broken functions, errors - there are workarounds Status:Fix Proposed A issue that has a PR related to it that provides a possible resolution Type: Bug Bugs within the core SuiteCRM codebase
Projects
None yet
Development

No branches or pull requests

2 participants