feat(web): Organization published material, request-response matching #7161

RunarVestmann · 2022-04-13T12:35:50Z

Organization published material, request-response matching

What

Matched what was searched with what got returned by comparing the hashed input with what the response gave us

Why

If the communication between the front- and backend suddenly lags then the UI on the page might be out of sync with what got returned so this change is to circumvent that

Checklist:

I have performed a self-review of my own code
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
Formatting passes locally with my changes
I have rebased against main before asking for a review

codecov · 2022-04-13T13:02:32Z

Codecov Report

Merging #7161 (d4ee586) into main (5e71d71) will increase coverage by 0.27%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #7161      +/-   ##
==========================================
+ Coverage   27.08%   27.36%   +0.27%     
==========================================
  Files        3192     3157      -35     
  Lines       53962    53334     -628     
  Branches    13327    13116     -211     
==========================================
- Hits        14616    14595      -21     
+ Misses      39346    38739     -607

Flag	Coverage Δ
api	`6.38% <ø> (ø)`
application-system-api	`69.60% <ø> (ø)`
auth-admin-web	`2.47% <ø> (ø)`
cms-translations	`37.50% <ø> (ø)`
judicial-system-api	`1.32% <ø> (ø)`
judicial-system-backend	`44.16% <ø> (+0.02%)`	⬆️
services-user-notification	`44.91% <ø> (ø)`
web	`5.60% <0.00%> (-0.08%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

github-actions · 2022-04-13T13:04:05Z

Affected services are: judicial-system-api,judicial-system-backend,application-system-api,api,services-user-notification,services-personal-representative-public,services-personal-representative,services-auth-public-api,air-discount-scheme-backend,financial-aid-api,services-auth-api,services-auth-admin-api,download-service,air-discount-scheme-api,services-search-indexer,financial-aid-backend,services-xroad-collector,icelandic-names-registry-backend,services-endorsements-api,services-user-profile,services-documents,judicial-system-xrd-api,reference-backend,skilavottord-ws,github-actions-cache,gjafakort-api,judicial-system-scheduler,gjafakort-application,gjafakort-queue-listener,financial-aid-web-veita,judicial-system-web,web,auth-admin-web,skilavottord-web,air-discount-scheme-web,gjafakort-web,financial-aid-web-osk,application-system-form,service-portal,island-ui-storybook,contentful-translation-extension,system-e2e,external-contracts-tests,
Feature deployment successful! Access your feature here:

cypress · 2022-04-13T13:18:05Z

Test summary

1 • 0 • 0 • 0 • 0

Run details


Project	island.is
Status	Passed
Commit	`8bae5b5` ℹ️
Started	Apr 19, 2022 2:46 PM
Ended	Apr 19, 2022 2:57 PM
Duration	11:11 💡
OS	Linux Ubuntu - 20.04
Browser	Electron 91

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

kristofer-lp8f

A few questions:

Is this a problem?
Is it worth the extra complexity?
Have you taken a look at debounce for delaying searching?
Have you taken a look at invalidating previous HTTP requests?
Have you considered relying on timestamps instead?

This seems like over-engineering for something I see not big a deal.

RunarVestmann · 2022-04-20T13:43:42Z

We are trying to prevent this from being a possible problem (for example this was an issue for the Sýslumenn auctions where the data was not reflecting your search string since the backend wasn't answering at a consistant enough rate)
It's not too much extra complexity since it's just one more string value to keep track of
I am debouncing
I don't see a simple way to do that since I'm using the useQuery hook
I could use timestamps instead of hashing the input since all I really need is a simple comparison between requests and responses

stefanhar · 2022-04-22T12:44:32Z

We are trying to prevent this from being a possible problem (for example this was an issue for the Sýslumenn auctions where the data was not reflecting your search string since the backend wasn't answering at a consistant enough rate)

It's not too much extra complexity since it's just one more string value to keep track of

I am debouncing

I don't see a simple way to do that since I'm using the useQuery hook

I could use timestamps instead of hashing the input since all I really need is a simple comparison between requests and responses

I believe there is already handling for some race conditions within apollo client.
Might there be some other reason we are seeing a weird result here?
Does the issue persist if fetchPolicy is set to network-only?

kristofer-lp8f · 2022-04-22T13:51:39Z

Some resources I've stumbled upon:

RunarVestmann · 2022-04-22T14:35:37Z

The implementation works fine without this change. This is just something a colleague pointed out and we added in case there is a halt in network communication. We could either just bail on this change and I'll close the PR or I could maybe do something a bit simpler like sending a timestamp instead of hashing the request. What do you guys think?

kristofer-lp8f · 2022-04-22T14:41:28Z

If this is a real problem or will be in the foreseeable future, I'd prefer the timestamp version. If not, I vote to close this PR.

Matched the response with what was requested

31669af

RunarVestmann added automerge Merge this PR as soon as all checks pass deploy-feature Deploys features to dev labels Apr 13, 2022

Used hash instead of comparing entire object

d4ee586

RunarVestmann marked this pull request as ready for review April 19, 2022 14:35

RunarVestmann requested review from a team as code owners April 19, 2022 14:35

RunarVestmann force-pushed the feature/organization-published-material-request-response-matching branch from 8952d13 to d4ee586 Compare April 19, 2022 14:41

kristofer-lp8f reviewed Apr 20, 2022

View reviewed changes

RunarVestmann closed this Apr 22, 2022

RunarVestmann deleted the feature/organization-published-material-request-response-matching branch April 22, 2022 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(web): Organization published material, request-response matching #7161

feat(web): Organization published material, request-response matching #7161

RunarVestmann commented Apr 13, 2022 •

edited

Loading

codecov bot commented Apr 13, 2022 •

edited

Loading

github-actions bot commented Apr 13, 2022

cypress bot commented Apr 13, 2022 •

edited

Loading

kristofer-lp8f left a comment

RunarVestmann commented Apr 20, 2022 •

edited

Loading

stefanhar commented Apr 22, 2022

kristofer-lp8f commented Apr 22, 2022

RunarVestmann commented Apr 22, 2022

kristofer-lp8f commented Apr 22, 2022

feat(web): Organization published material, request-response matching #7161

feat(web): Organization published material, request-response matching #7161

Conversation

RunarVestmann commented Apr 13, 2022 • edited Loading

Organization published material, request-response matching

What

Why

Checklist:

codecov bot commented Apr 13, 2022 • edited Loading

Codecov Report

github-actions bot commented Apr 13, 2022

cypress bot commented Apr 13, 2022 • edited Loading

Test summary

Run details

kristofer-lp8f left a comment

Choose a reason for hiding this comment

RunarVestmann commented Apr 20, 2022 • edited Loading

stefanhar commented Apr 22, 2022

kristofer-lp8f commented Apr 22, 2022

RunarVestmann commented Apr 22, 2022

kristofer-lp8f commented Apr 22, 2022

RunarVestmann commented Apr 13, 2022 •

edited

Loading

codecov bot commented Apr 13, 2022 •

edited

Loading

cypress bot commented Apr 13, 2022 •

edited

Loading

RunarVestmann commented Apr 20, 2022 •

edited

Loading