If worklist is restarting, don't set new sort comparator

[nmajor25]

Shortcut Story ID: [sc-50540]

[nmajor25]

I can't reproduce this bug in my dev environment, so can't tell if this actually fixes the issue.

Also still contemplating how to test this.

[paulfalgout]

Here's the test, but isRestarting doesn't fix the issue:

    cy
      .intercept('GET', '/api/flows*', { delay: 1000, body: { data: [] } })
      .visit('/worklist/owned-by');

    cy
      .get('[data-date-filter-region]')
      .should('contain', 'Added:')
      .should('contain', 'This Month')
      .click();

    cy
      .get('.app-frame__pop-region')
      .contains('Last Week')
      .click();

    cy
      .get('.worklist-list__filter-sort')
      .click()
      .get('.picklist')
      .contains('Patient Last: A')
      .click();

[nmajor25]

What about checking that the list view has a collection before setting the comparator?

Like:

  onChangeStateSort() {
    if (!this.isRunning()) return;

    const listView = this.getChildView('list');

    if (listView.collection) {
      listView.setComparator(this.getComparator());
    }
  },

That seems to work...

[cypress]

Passing run #6366 ↗︎

0

281

0

0

0

Details:

Fix sync bug in the `/base/app.js`
Project: RoundingWell Care Ops Frontend	Commit: d20ce66977
Status: Passed	Duration: 02:56 💡
Started: May 9, 2024 3:39 PM	Ended: May 9, 2024 3:42 PM

Review all test suite changes for PR #1259 ↗︎

[coveralls]

Pull Request Test Coverage Report for Build cdb37ccb-08f4-4b90-973a-bd4d4ba2477f

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall first build on bugfix/worklist-filter at 99.91%

---

Totals
Change from base Build 965aa496-a373-4df6-a574-0334073fadb4:	99.9%
Covered Lines:	5941
Relevant Lines:	5945

---

💛 - Coveralls

[paulfalgout]

Here's the test, but isRestarting doesn't fix the issue:

    cy
      .intercept('GET', '/api/flows*', { delay: 1000, body: { data: [] } })
      .visit('/worklist/owned-by');

    cy
      .get('[data-date-filter-region]')
      .should('contain', 'Added:')
      .should('contain', 'This Month')
      .click();

    cy
      .get('.app-frame__pop-region')
      .contains('Last Week')
      .click();

    cy
      .get('.worklist-list__filter-sort')
      .click()
      .get('.picklist')
      .contains('Patient Last: A')
      .click();

[paulfalgout]

OK so.. this fixed seemed oddly unnecessary.. how was it that the app was running.. wasn't restarting and still had the preloader running.

It turns out the issue was deeper and it might subsequently also solve the issue we're seeing in the program action bug. This'll actually be an odd place to test this.. Really at some point we need to add component tests for the base classes. But I think we can keep this test and just add a comment that links to this PR and a message about moving it to a component test.

So in base/app.js it was setting loading to false, but that fetchId check prevents older fetches from resolving.. so.. if you make two requests one after another where the 2nd should take precedent.. the first one clears the loading flag which modifies running..

Currently:

  onSyncData(fetchId, options, args = []) {
    this._isLoading = false;
    if (!this.isRunning() || this._fetchId !== fetchId) return;

    this.finallyStart.call(this, options, ...args);
  },
  triggerSyncFail(fetchId, options, ...args) {
    this._isLoading = false;
    if (!this.isRunning() || this._fetchId !== fetchId) return;

    this.triggerMethod('fail', options, ...args);
  },

Change to: (notice the switch from isRunning() to _isRunning)

onSyncData(fetchId, options, args = []) {
    if (!this._isRunning || this._fetchId !== fetchId) return;

    this._isLoading = false;

    this.finallyStart.call(this, options, ...args);
  },
  triggerSyncFail(fetchId, options, ...args) {
    if (!this._isRunning || this._fetchId !== fetchId) return;

    this._isLoading = false;

    this.triggerMethod('fail', options, ...args);
  },

[shadowhand]

@paulfalgout that explanation makes a ton of sense. My spidey senses were tingling when I saw this story because it seemed like something we would have noticed before, as the particular trigger is relatively common.

[nmajor25]

@paulfalgout interesting... I updated the PR to your solution and it seems to fix this worklist bug (via the test passing).

But it doesn't fix the program action sidebar bug in #1260.