Fix(planning): Avoid double AJAX call to get_events on FullCalendar initiazation

[stonebuzz]

Checklist before requesting a review

Please delete options that are not relevant.

• I have read the CONTRIBUTING document.
• I have performed a self-review of my code.
• I have added tests that prove my fix is effective or that my feature works.
• This change requires a documentation update.

Description

• It fixes !37230

During the plugin initialization, we observe two AJAX calls made to load events.

The first one results from the calendar initialization,

while the second is triggered by the computation and loading of the last default view.

trigger by this

        // Load the last known view only if it is valid (else load default view)
        const view = this.calendar.isValidViewType(options.default_view) ?
            options.default_view :
            default_options.default_view;
        this.calendar.changeView(view);

This second call is redundant, as the default view has already been computed server-side (in PHP) and does not need to be reloaded / check on the client side.

            $options = [
                'full_view'    => true,
                'default_view' => $_SESSION['glpi_plannings']['lastview'] ?? 'timeGridWeek',  // HERE
                'license_key'  => $scheduler_key,
                'resources'    => self::getTimelineResources(),
                'now'          => date("Y-m-d H:i:s"),
                'can_create'   => PlanningExternalEvent::canCreate(),
                'can_delete'   => PlanningExternalEvent::canPurge(),
            ];

Screenshots (if appropriate):

[AdrienClairembault]

See #16439, the lines you removed prevent a fatal error when the current view is invalid (e.g. a view from a plugin that was since disabled).

[AdrienClairembault]

If you want to improve this, maybe the options could be checked on the PHP side.

[stonebuzz]

If you want to improve this, maybe the options could be checked on the PHP side.
Voici une version améliorée, corrigée, complétée, rendue professionnelle et traduite en anglais de ton message :

A verification on the PHP side is not possible because the resourceTimeline plugin is always loaded by FullCalendar. It is then up to FullCalendar, on the client side, to check whether the corresponding library is actually available and whether a valid license_key has been provided (see the PHP code in the AdvancedDashboard plugin):

$PLUGIN_HOOKS['planning_scheduler_key']['advancedplanning'] = function () {
    return 'GPL-My-Project-Is-Open-Source';
};

GLPI (on the PHP side) has no knowledge of which JavaScript plugins are effectively loaded. Furthermore, plugin names often differ from the view types used by the user. For instance, the timeGridWeek view is provided by the timeGrid plugin.
I have therefore refactored the view check logic to fall back to the default view only when the currently requested view is Invalid.

[AdrienClairembault]

Did you try it with the plugin to make sure the issue doesn't happen ?

Because in #16439 defaultView is not set in the constructor so we have no choice but to call this.calendar.changeView(view).

I noticed that it was added back in #16788, which probably bring back the original issue again...

[stonebuzz]

Indeed !!

I did a small refactor.

I removed the use of defaultView from constructor, which caused the following error when the Advanced Planning plugin is disabled and the resourceWeek view type is no longer available:
👉 #16439

The idea is to handle the view only after initialization.

If the requested view is valid, it will be applied. Otherwise, it will fall back to the default view.

[AdrienClairembault]

It seems to me that with the changes, If options.default_view is valid, it is never applied.

[AdrienClairembault]

See last comment.

[stonebuzz]

last commit should solve the problems

[stonebuzz]

The latest commit also removes code that caused the get_events AJAX endpoint to be called twice.

// Save the original datesRender function, which is later overwritten by the "selectable" option
const originalDatesRender = GLPIPlanning.calendar.getOption('datesRender');
GLPIPlanning.calendar.setOption('selectable', false);
window.setTimeout(function() {
    GLPIPlanning.calendar.setOption('selectable', true);
    originalDatesRender(info); // Restore datesRender
}, 500);

This code was only intended to temporarily disable and then re-enable date selection controls.
Calling originalDatesRender ensured that navigation buttons were properly redrawn.

However, the current implementation (since #19184) triggered a duplicate call to get_events when switching views:

View change -> get_events -> render -> datesRender -> refresh -> get_events

Since events are added via a modal, the date selection controls aren't accessible at that point.
Therefore, there's no need to disable them temporarily, which justifies the removal of this code.

[AdrienClairembault]

Is there a support ticket linked to this issue ?

I am not against it but there are now a lot of changes on a part of GLPI that is not covered by tests.
It would be safer to change the target to main.

[trasher]

Is there a support ticket linked to this issue ?

PR description say it's related to !37230

I am not against it but there are now a lot of changes on a part of GLPI that is not covered by tests. It would be safer to change the target to main.

That does not seems so much changes; but since this is only javascript, I can be wrong... :D