bryntum / support

An issues-only repository for the Bryntum project management component suite which includes powerful Grid, Scheduler, Calendar, Kanban Task Board and Gantt chart components all built in pure JS / CSS / TypeScript
https://www.bryntum.com
54 stars 6 forks source link

applyChangeset in event store triggers a sync #9294

Closed taauntik closed 3 months ago

taauntik commented 5 months ago

Forum post

To reproduce, paste the code snippet below into the scheduler pro demo code editor.

import { ProjectModel, EventModel, AjaxHelper, SchedulerPro, Timeline, StringHelper } from '../../build/schedulerpro.module.js?476743';
import shared from '../_shared/shared.module.js?476743';

const resources = [
    { id : 'r6', name : 'Gloria', company : 'Small rocket co' }
];
const events = [
    { id : 1000, name : 'Event 1', startDate: "2017-01-01", endDate: "2017-01-02" }
]
const assignments = [
    { id: 1, eventId: 1000, resourceId: 'r6' }
]

class EventModelA extends EventModel {
static get fields() {
    return [
      { name: "startDate" },
      { name: "endDate" },
      { name: "duration", persist: false },
    ];
  }

}

AjaxHelper.mockUrl('load', {
    responseText : JSON.stringify({
        success   : true,
        resources : { rows : resources },
        events : { rows : events },
        assignments : { rows: assignments },
    })
});

let count = 1000;

AjaxHelper.mockUrl('sync', function(url, params, options) {
    const body = JSON.parse(options.body);

console.log(body);

if (body.events.added) {
    body.events.added.forEach(added => {
        added.id = count++;
    });
}

return {
    responseText : JSON.stringify({
        success : true,
        events  : {
            rows : [
                ...(body.events?.added || []),
                ...(body.events?.updated || [])
            ],
            removed : [...(body.events?.removed || [])]
        }
    }),
    delay : 200
};
});

const schedulerPro = new SchedulerPro({
    appendTo          : 'container',
    resourceImagePath : '../_shared/images/users/',

features : {
    eventTooltip : true
},

columns : [
    { type : 'resourceInfo', text : 'L{Name}', width : 150 },
    { text : 'L{Company}', field : 'company', width : 150 }
],

project : {
    eventModelClass: EventModelA,
    loadUrl  : 'load',
    syncUrl  : 'sync',
    autoLoad : true,
    autoSync : true
},

startDate  : new Date('2017-01-01'),
endDate    : new Date('2017-01-02'),
viewPreset : 'hourAndDay'
});

Once the demo loads, paste the below code snippet in browser's console and press return:

const scheduler = bryntum.query("SchedulerPro");
scheduler.project.eventStore.applyChangeset({
    updated: [{ id: 1000, startDate: "2017-01-01T10:00:00+05:00", endDate: "2017-01-01T12:00:00+05:00" }]
})

You will see sync request logged to the console

Video: https://www.loom.com/share/7dd8fd2a81b84a53808ce48f629afa8f

bmblb commented 4 months ago

Sync request may be valid here, if it includes only calculated fields, like duration, effort, etc. Because added event is not complete. However, if sync request contains this event as added, it is a problem indeed

ghulamghousdev commented 4 months ago

@bmblb I believe sync request here is valid here because there is no new added event and eventStore.changes.modified[0].modificationDataToWrite has startDate and endDate as they are being updated for the event. So a sync request is triggered to sync them with the backend which is totally valid as they are being changed on the frontend.

Screenshot 2024-07-09 at 2 06 16 PM
muhammad-faisal101 commented 4 months ago

@bmblb @ghulamghousdev In your case a sync would be valid since the duration field is being updated. But in the demo i provided https://forum.bryntum.com/viewtopic.php?p=147557#p147557, I have kept the { name: "duration", persist: false }

@ghulamghousdev In my case the data is being generated on the backend and we need to sync it to the front end. for this, we are using the applyChangeSet function (as stated in the docs Core.data.Store#function-applyChangeset) but it is calling the sync request again and causing problems for us.

ghulamghousdev commented 4 months ago

@muhammad-faisal101 Even if { name: "duration", persist: false } is set, startDate and endDate have persist: true and the response you are applying have different dates then the current dates of the event in the store. So that is definitely gonna trigger a sync call. There are couple of things that you can do here is to avoid that call. You can set persist to false on both the start and end date. This is gonna make sure no sync call is triggered as no changes are going to be tracked. Another option would be to set autoSync: false but the changes are still gonna be tracked and will be available on projectModel.changes. You can clear those changes but there's a risk of clearing out any changes that are made during the process of applyChangeSet. In a nutshell, this ticket is invalid.

muhammad-faisal101 commented 4 months ago

@ghulamghousdev that would make sense. But in my case, I am manually providing the start and end date (like scheduler.project.eventStore.applyChangeset({ updated: [{ id: 1000, startDate: "2017-01-01T10:00:00+05:00", endDate: "2017-01-01T12:00:00+05:00" }] })). Both the start and end dates are already updated on the backend. I need to make sure that the front end is updated as well.

muhammad-faisal101 commented 4 months ago

If you look at the documentation, the applyChangeset function is used to apply changes coming from the backend. if these changes are already present on the backend, why create an API call with the same changes again?

image
ghulamghousdev commented 4 months ago

If you look at the documentation, the applyChangeset function is used to apply changes coming from the backend. if these changes are already present on the backend, why create an API call with the same changes again?

Well both options sync or do not sync makes sense here. We need a callout on this from @isglass , who is on vacations this week. So I am pausing any further development on this and will discuss this with him once he's back and update the ticket accordingly.

bmblb commented 4 months ago

At this point I am not even sure how applyChangeset should behave. I can see logical explanation for both triggering and not triggering sync. Provided applyChangeset is used by other API and should be as simple as possible, I lean a little towards keeping current behavior: everything behaves as is, problem of applying and committing changes is solved on the project level by applyProjectChanges API, which uses this exact method.

isglass commented 3 months ago

Resolved by updating docs, pointing to applyProjectChanges for when it should not sync any resulting changes