Skip to content

Conversation

oguzhankoral
Copy link
Member

It is a first pass. And tested via console, it does what it supposed to do.

@oguzhankoral oguzhankoral requested a review from didimitrie March 14, 2025 19:57
Copy link

linear bot commented Mar 14, 2025

CNX-1417 Remove account

Copy link

codecov bot commented Mar 14, 2025

Codecov Report

Attention: Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.

Project coverage is 15.41%. Comparing base (be63b4a) to head (ad15bf8).
Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
...peckle.Connectors.DUI/Models/DocumentModelStore.cs 0.00% 12 Missing ⚠️
.../Speckle.Connectors.DUI/Bindings/AccountBinding.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #680      +/-   ##
==========================================
- Coverage   15.45%   15.41%   -0.05%     
==========================================
  Files         230      230              
  Lines        4606     4619      +13     
  Branches      551      554       +3     
==========================================
  Hits          712      712              
- Misses       3866     3879      +13     
  Partials       28       28              

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@didimitrie didimitrie left a comment

Choose a reason for hiding this comment

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

Is this about removing models in bulk or around removing accounts, or both?

@@ -67,6 +67,8 @@ ITopLevelExceptionHandler topLevelExceptionHandler

public void RemoveModel(ModelCard model) => _store.RemoveModel(model);

public void RemoveModels(List<ModelCard> models) => _store.RemoveModels(models);
Copy link
Member

Choose a reason for hiding this comment

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

See corollary comment in FE PR: specklesystems/speckle-server#4203 (comment)

tl;dr: do we need this vs. removing models in a for loop?

Copy link
Member

Choose a reason for hiding this comment

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

got my reply in the other pr

@didimitrie didimitrie self-requested a review March 15, 2025 18:19
@oguzhankoral oguzhankoral merged commit 50807b1 into dev Mar 15, 2025
5 checks passed
@oguzhankoral oguzhankoral deleted the oguzhan/cnx-1417-remove-account branch March 15, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants