Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

feat: adding certificate delivery subscriptions #328

Merged
merged 2 commits into from
Oct 18, 2023
Merged

feat: adding certificate delivery subscriptions #328

merged 2 commits into from
Oct 18, 2023

Conversation

Freyskeyd
Copy link
Member

Description

Wire the broadcast delivered certificates notification into the new GraphQL subscriptions.

An example query:
Screenshot 2023-10-05 at 15 26 17

A Websocket is used to expose and push messages to the frontend.

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@Freyskeyd Freyskeyd requested a review from a team as a code owner October 5, 2023 13:30
@Freyskeyd Freyskeyd requested review from dvdplm and JDawg287 and removed request for a team October 5, 2023 13:30
@Freyskeyd Freyskeyd force-pushed the feature/tp-634 branch 3 times, most recently from bc0ec68 to 5e2346f Compare October 5, 2023 15:08
gruberb
gruberb suggested changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

👍

crates/topos-tce-api/src/graphql/query.rs Outdated Show resolved Hide resolved
crates/topos-tce-api/src/graphql/query.rs Outdated Show resolved Hide resolved
@Freyskeyd Freyskeyd force-pushed the feature/tp-681 branch 4 times, most recently from 25da63e to a6f376b Compare October 6, 2023 10:50
Base automatically changed from feature/tp-681 to main October 9, 2023 13:07
dvdplm
dvdplm approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Left some comments and questions but overall this looks like it's ready to go.

crates/topos-tce-api/src/graphql/filter.rs Show resolved Hide resolved
crates/topos-tce-api/src/graphql/query.rs Outdated Show resolved Hide resolved
crates/topos-tce-api/src/runtime/commands.rs Outdated Show resolved Hide resolved
crates/topos-tce-api/src/runtime/mod.rs Show resolved Hide resolved
crates/topos-tce-api/src/runtime/mod.rs Outdated Show resolved Hide resolved
crates/topos-tce-api/src/graphql/routes.rs Show resolved Hide resolved
crates/topos-api/src/graphql/filter.rs Outdated Show resolved Hide resolved
@Freyskeyd Freyskeyd force-pushed the feature/tp-634 branch 2 times, most recently from edaeb44 to 63018c3 Compare October 9, 2023 16:20
@codecov
Copy link

codecov bot commented Oct 9, 2023 •
edited
Loading

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (aa21dcc) 60.73% compared to head (05c0255) 60.79%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #328      +/-   ##
==========================================
+ Coverage   60.73%   60.79%   +0.06%     
==========================================
  Files         231      232       +1     
  Lines       12085    12135      +50     
==========================================
+ Hits         7340     7378      +38     
- Misses       4745     4757      +12
Files Coverage Δ
crates/topos-api/src/graphql/certificate.rs 95.45% <100.00%> (-0.70%) ⬇️
crates/topos-api/src/graphql/filter.rs 100.00% <100.00%> (ø)
crates/topos-tce-api/src/graphql/builder.rs 100.00% <100.00%> (ø)
crates/topos-tce-api/src/graphql/tests.rs 93.18% <100.00%> (-0.94%) ⬇️
crates/topos-tce-api/src/runtime/commands.rs 0.00% <ø> (ø)
crates/topos-tce-api/src/stream/mod.rs 69.69% <ø> (ø)
crates/topos-tce-api/tests/runtime.rs 92.74% <ø> (ø)
crates/topos-tce-storage/src/tests/mod.rs 97.56% <ø> (ø)
crates/topos-api/src/graphql/subnet.rs 66.66% <57.14%> (+4.16%) ⬆️
crates/topos-tce-api/src/graphql/routes.rs 38.46% <0.00%> (-30.29%) ⬇️
... and 2 more

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dvdplm
Copy link
Contributor

dvdplm commented Oct 11, 2023

@gruberb Can you re-review please and let us know if you still have concerns?

@dvdplm dvdplm closed this Oct 11, 2023
@dvdplm dvdplm reopened this Oct 11, 2023
gruberb
gruberb approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

👍

@Freyskeyd
feat: adding certificate delivery subscriptions
2ad844e 
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
@Freyskeyd
feat: adding subnet graphql test
05c0255 
Signed-off-by: Simon Paitrault <simon.paitrault@gmail.com>
@dvdplm dvdplm merged commit ef02f04 into main Oct 18, 2023
@dvdplm dvdplm deleted the feature/tp-634 branch October 18, 2023 06:39
@sebastiendan sebastiendan mentioned this pull request Nov 27, 2023
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dvdplm dvdplm dvdplm approved these changes

@atanmarko atanmarko atanmarko approved these changes

@gruberb gruberb gruberb approved these changes

@JDawg287 JDawg287 Awaiting requested review from JDawg287 JDawg287 is a code owner automatically assigned from topos-protocol/protocol

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Freyskeyd @dvdplm @gruberb @atanmarko