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.

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

Separate "define" and "publish" for contract interfaces, and allow deleting unpublished interfaces #1279

Merged
merged 4 commits into from
May 30, 2023

Conversation

awrichar
Copy link
Contributor

@awrichar awrichar commented Apr 18, 2023

Part of the fix for #1220
In a chain with #1261
Depends on hyperledger/firefly-common#64

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2023

Codecov Report

Merging #1279 (0ffe1b0) into main (56b8d2f) will decrease coverage by 0.03%.
The diff coverage is 97.37%.

❗ Current head 0ffe1b0 differs from pull request most recent head e54db7c. Consider uploading reports for the commit e54db7c to get more accurate results

@@             Coverage Diff             @@
##              main    #1279      +/-   ##
===========================================
- Coverage   100.00%   99.97%   -0.03%     
===========================================
  Files          310      312       +2     
  Lines        20960    21123     +163     
===========================================
+ Hits         20960    21117     +157     
- Misses           0        6       +6     
Impacted Files Coverage Δ
...erver/route_get_contract_interface_name_version.go 100.00% <ø> (ø)
internal/apiserver/routes.go 100.00% <ø> (ø)
internal/definitions/sender.go 100.00% <ø> (ø)
internal/database/sqlcommon/ffi_sql.go 96.51% <94.82%> (-3.49%) ⬇️
...ernal/apiserver/route_delete_contract_interface.go 100.00% <100.00%> (ø)
...apiserver/route_post_contract_interface_publish.go 100.00% <100.00%> (ø)
...nal/apiserver/route_post_new_contract_interface.go 100.00% <100.00%> (ø)
internal/contracts/manager.go 100.00% <100.00%> (ø)
internal/definitions/handler_contracts.go 100.00% <100.00%> (ø)
internal/definitions/sender_contracts.go 100.00% <100.00%> (ø)

@awrichar awrichar changed the title Separate "define" and "publish" for contract interfaces Separate "define" and "publish" for contract interfaces, and allow deleting unpublished interfaces May 24, 2023
Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

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

Looks great!

Only thing I noticed was that coverage dropped in the SQL package with those updates.
Happy to leave it with you whether to close that before merge.

case iface == nil:
return wrapSendError(i18n.NewError(ctx, coremsgs.MsgContractInterfaceNotFound, pool.Interface.ID))
case !iface.Published:
return wrapSendError(i18n.NewError(ctx, coremsgs.MsgContractInterfaceNotPublished, pool.Interface.ID))
Copy link
Contributor

Choose a reason for hiding this comment

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

Great to see this case covered 👍

@awrichar
Copy link
Contributor Author

I double-checked coverage on ffi_sql.go locally and it seems to be covered - so writing off the bot as a mistake. I'll check again after merging though.

@awrichar awrichar merged commit 301fbc3 into hyperledger:main May 30, 2023
@awrichar awrichar deleted the deletethings2 branch May 30, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants