Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: replace old Status index with new StatusCreatedAt one #4691

Merged
merged 3 commits into from
Dec 2, 2024
Merged

feat: replace old Status index with new StatusCreatedAt one #4691

merged 3 commits into from
Dec 2, 2024

Conversation

craigzour
Copy link
Contributor

@craigzour craigzour commented Nov 21, 2024 •
edited
Loading

Summary | Résumé

Part 5 of #4287

  • Replaces use of old Status index with new StatusCreatedAt one
  • Loads the RemovalDate piece of information for each submission listed under Signed off for removal tab in an asynchronous way. This is due to the new index not returning the RemovalDate data in its projected attributes.

Tests

  • On the response page (in all tabs): make sure you can still see all form submissions you were getting before testing the new index. The only difference should be that items are now ordered by CreatedAt date. It means that the oldest response should be at the top of the list.
  • On the response page (tab: Signed off for removal): items should asynchronously load their "Next step" piece of information.

Testing options:

@craigzour craigzour self-assigned this Nov 21, 2024
@craigzour craigzour changed the title Feature/replace old status index with new status created at index feat: replace old status index with new status created at index Nov 21, 2024
@craigzour craigzour changed the title feat: replace old status index with new status created at index feat: replace old Status index with new StatusCreatedAt one Nov 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

🧪 Review environment

https://4oso5apsmwc6ktdyauecgpwoki0xsumj.lambda-url.ca-central-1.on.aws/

@craigzour craigzour force-pushed the feature/replace-old-status-index-with-new-status-created-at-index branch 4 times, most recently from 3f16502 to 0279082 Compare November 27, 2024 16:44
@craigzour craigzour mentioned this pull request Nov 27, 2024
Closed
@craigzour craigzour force-pushed the feature/replace-old-status-index-with-new-status-created-at-index branch from 0279082 to f7d4435 Compare December 2, 2024 14:31
@craigzour craigzour marked this pull request as ready for review December 2, 2024 14:31
timarney
timarney reviewed Dec 2, 2024
View reviewed changes
...form administration)/form-builder/[id]/responses/[[...statusFilter]]/components/NextStep.tsx
}, []);

if (label === null) {
return <Skeleton count={1} className="my-4 ml-4 w-[300px]" />;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

timarney
timarney previously approved these changes Dec 2, 2024
View reviewed changes
Copy link
Member

@timarney timarney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this + all the previous steps to get to this point.

thiessenp-cds
thiessenp-cds approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@thiessenp-cds thiessenp-cds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the responses tabs and the submissions are in order.
Testing the download formats, the submissions could be out of order. I added a quick sort() call on that to make sure they'd be in order. Kind of out scope of this PR but nice to fix now when it's obvious.
Nice work! LGTM :)

@craigzour craigzour merged commit 9b1caf9 into main Dec 2, 2024
13 checks passed
@craigzour craigzour deleted the feature/replace-old-status-index-with-new-status-created-at-index branch December 2, 2024 20:38
@sre-read-write sre-read-write bot mentioned this pull request Dec 2, 2024
Merged
@craigzour craigzour mentioned this pull request Dec 2, 2024
Closed
craigzour added a commit that referenced this pull request Dec 3, 2024
@craigzour
Revert "feat: replace old Status index with new StatusCreatedAt one (#…
8cdc9b8 
…4691)"

This reverts commit 9b1caf9.
craigzour added a commit that referenced this pull request Dec 3, 2024
@craigzour
Revert "feat: replace old Status index with new StatusCreatedAt one (#…
78a8457 
…4691)" (#4761)

This reverts commit 9b1caf9.
craigzour added a commit that referenced this pull request Dec 3, 2024
@craigzour
Reapply "feat: replace old Status index with new StatusCreatedAt one (#…
0a7038b 
…4691)" (#4761)

This reverts commit 78a8457.
craigzour added a commit that referenced this pull request Dec 3, 2024
@craigzour
Reapply "feat: replace old Status index with new StatusCreatedAt one (#…
ce1a6a4 
…4691)" (#4761)

This reverts commit 78a8457.
craigzour added a commit that referenced this pull request Dec 10, 2024
@craigzour
Reapply "feat: replace old Status index with new StatusCreatedAt one (#…
a905145 
…4691)" (#4761)

This reverts commit 78a8457.
craigzour added a commit that referenced this pull request Dec 10, 2024
@craigzour
Reapply "feat: replace old Status index with new StatusCreatedAt one (#…
c031a4a 
…4691)" (#4761) (#4763)

* Reapply "feat: replace old Status index with new StatusCreatedAt one (#4691)" (#4761)

This reverts commit 78a8457.

* fix DynamoDB request configuration when no Status#CreatedAt need to be passed in
craigzour added a commit that referenced this pull request Dec 11, 2024
@craigzour
Revert "Reapply "feat: replace old Status index with new StatusCreate…
97c7b98 
…dAt one (#4691)" (#4761) (#4763)"

This reverts commit c031a4a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thiessenp-cds thiessenp-cds thiessenp-cds approved these changes

@timarney timarney timarney left review comments

@bryan-robitaille bryan-robitaille Awaiting requested review from bryan-robitaille

Assignees

@craigzour craigzour

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@craigzour @timarney @thiessenp-cds