Problem/Motivation

When calling getObject it should be possible to indicate that new data should be fetched, even if the object already exists in the store.

Proposed resolution

Could indicate a required fetch with something like:

await store.getObject({
    objectName: 'recipes',
    id: '912e092f-a7d5-41ae-9e92-e23ffa357b28',
    fetch: true
})

Maybe 'refresh' is a better parameter name in case the method to get data changes in the future?

Issue fork drupal_state-3240885

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Toggle MR activity (6 comments)

brianperry created an issue. See original summary.

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Pantheon commented
Priority: Normal » Major

jayhuskins made their first commit to this issue’s fork.

jayhuskins opened merge request !40

Add refresh option to getObject

Closes #3240885

jayhuskins’s picture

jayhuskins
he/they
CreditAttribution: jayhuskins at Redfin Solutions, LLC commented

@brianperry here is a small adjustment that adds a refresh parameter to the getObject function. Should this parameter be added to getObjectByPath as well? I do not need that functionality, but it could be nice to have.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
CreditAttribution: chrisfromredfin at Redfin Solutions, LLC commented
Status: Active » Needs review

I took a look at this, and will mark as NR - not comfortable RTBC until you answer the question re: getObjectByPath. Thanks!

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Pantheon commented

Thanks for the contribution @jayhuskins (and the nudge @chrisfromredfin)! Haven't had a chance to take a close look at the MR but hope to do that soon. In the meantime, I do think adding the equivalent functionality to getObjectByPath would be useful.

jayhuskins’s picture

jayhuskins
he/they
CreditAttribution: jayhuskins at Redfin Solutions, LLC commented

@brianperry Okay I added the refresh functionality to getObjectByPath as well.

coby.sher’s picture

coby.sher CreditAttribution: coby.sher at Pantheon commented

Thanks @jayhuskins! Could you add tests for this functionality to drupalState.test.ts and getobjectByPath.test.ts? I think it will be good to go after that.

jayhuskins’s picture

jayhuskins
he/they
CreditAttribution: jayhuskins at Redfin Solutions, LLC commented
Status: Needs review » Reviewed & tested by the community

@coby.sher the tests are up!

coby.sher’s picture

coby.sher CreditAttribution: coby.sher at Pantheon commented

@jayhuskins thank you! LGTM. I added some docs and an example in main.ts. Will have @brianperry take a look but it should be good to go.

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Pantheon commented
Status: Reviewed & tested by the community » Fixed

Merged and released as 2.7.0 https://www.npmjs.com/package/@gdwc/drupal-state/v/2.7.0

Thanks a ton for the contribution @jayhuskins and thanks for the review @chrisfromredfin and @coby.sher!

I think there is a lot that we could do in the future to better handle merging and updating data that already exists in the store, but this feature will go a long way in the meantime.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.