[Bugfix] Multiple calls of editableFeatures method breaks php

[mind84]

When a popup is displayed the system checks first whether the user can edit the feature or not, so as to configure the lizmap-feature-toolbar HTML element correctly.

At time being, if the layer is filtered by user (Attribute filtering tab in Lizmap plugin), the system queries all the features of the layers and then filters the entire list by feature id to perform the check.

lizmap-web-client/lizmap/modules/lizmap/lib/Request/WMSRequest.php

Line 785 in 25ba42b

$editableFeatures = $qgisLayer->editableFeatures();

This operation is executed for every feature on popup, even if features belonging to same layer.

If the layer has a lot of feature then this operation easily leads to php exceptions (max_execution_time on master branch with jsonmachine, memory error for Lizmap <= 3.8 )

I didn't use the isFeatureEditable method (which seems suitable for this job) because it requires a feature object that you can't get from a WMS request (like the Popup request) so, as a first solution, I chose to parameterize the editableFeatures function.

Backport to 3.8 and 3.9, if possible

Funded by Faunalia

[rldhont]

Hi @mind84,

Thanks for your contribution, if you want to update editableFeatures, I am much in favor of

    /**
     * Get the layer editable features.
     * Used client-side (JS) to fetch the features which are editable by the authenticated user
     * when there is a filter by login (or by polygon). This allows to deactivate the editing icon
     * for the non-editable features inside the popup and attribute table.
      * @param array<string, string> [$wfsParams] Extra WFS parameters to filter the layer : FEATUREID or EXP_FILTER could be use
     *
     * @return array Data containing the status (restricted|unrestricted) and the features if restricted
     */
    public function editableFeatures($wfsParams = array())

In this case editableFeatures can be use in isFeatureEditable

[mind84]

@rldhont,

I leave the isFeatureEditable method refactoring for another PR. There is some shared logic between the two functions but they do quite different jobs so I'll have to work on that

[rldhont]

Thanks @mind84

[mind84]

@rldhont,

there a bunch of tests that fail, some might be related to these changes ( filter-layer-by-user.spec.js ), at least at first glance.

Looking closer, the errors seem more related to some interactions with the interface. Also, I can't reproduce these errors in my local development environment.

Should be worried about that?

Thanks!

[Gustry]

Should be worried about that?

We are first looking at QGIS 3.34 stack, which is green in your PR.
For QGIS 3.40, we are aware about false positive. Be sure you have an up to date branch on master, some tests about 3.40 are fixed already.

filter-layer-by-user.spec.js is already fixed with #5470