cap-js / change-tracking

CDS plugin providing out-of-the box support for automatic capturing, storing, and viewing of the change records of modeled entities.
https://cap.cloud.sap/docs
Apache License 2.0
20 stars 8 forks source link

Noticed changes in v1.0.7 #101

Closed navinkrishnan closed 1 month ago

navinkrishnan commented 3 months ago

Hi,

In the Incidents-app test case for change-tracking, after deleting the incident following select query resulted in 0 results. This test case is passing with @cap-js/change-tracking 1.0.6.

https://github.com/cap-js/incidents-app/blob/259abc1e41142f2d2f27d3c8cb09af0ee1e9aed1/xmpls/change-tracking.test.js#L125-L130

But checking with the upcoming release from the main branch of this repo (1.0.7), test case fails. I would like to understand, if this is the expected behavior ?

If yes, I'm proposing the following change in test case. Please confirm if this is meaningful test.

it('+ Test the status detail in ChangeView', async () => {
        const incidentChanges = await SELECT.from(ChangeView).where({
                entity: "sap.capire.incidents.Incidents",
                attribute: "status",
                keys: `ID=${draftId}`,
                modification: "delete"
            })
        expect(incidentChanges.length).to.equal(1);
      });
nkaputnik commented 3 months ago

Hello @navinkrishnan we did take a look at the test case, and we've seen that you already turned on the global switch to preserve deletes in line 88 of the test. This will naturally invalidate the test you mentioned. We suggest to either turn the global switch off again before running this particular test, or even better, test against both cases.

Best, Nick

Sv7enNowitzki commented 3 months ago

Hi @navinkrishnan

I confirmed that the global switch we added is named: preserveDeletes, however, the Property on line 84 mentioned before is PreserveChanges. I assume that this PreserveChanges has nothing to do with our global switch.

Based on this, I ran the test case in the incident-app and found that it has all passed (based on main branch code). In addition, after adding the global switch: preserveDeletes, the changelog is also consistent with our expected behavior.

So I want to make sure how do you reproduce this problem? Or could you give me some more specific information?

Best Regards, Wenjun

navinkrishnan commented 3 months ago

Issue is reproducible with combination of cds 8 and change-tracking: 1.0.7. The global switch preserveDeletes seems to not take effect. It always preserves the delete history. Provided sample to @Sv7enNowitzki

nkaputnik commented 3 months ago

Hello @navinkrishnan it seems that CDS 8 has made changes to internal API's which break the behavior of how we detect deletes. So, we will need to fix this in the plugin, please do not change the tests, since they reflect the expected behavior.

Best, Nick

nkaputnik commented 2 months ago

Hello @navinkrishnan we released a new version today, can you please take a look and see whether the tests now behave as intended?

Best, Nick