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

[ABW-4081] Security shields list #1336

Merged
merged 19 commits into from
Feb 18, 2025

Conversation

giannis-rdx
Copy link
Contributor

@giannis-rdx giannis-rdx commented Feb 17, 2025

Description

This PR adds a list of security shields to the “Security Shields” screen. Each Security Shield card currently contains the following information:

  • name of the shield
  • linked entities (Assigned to:)
    Other operations in the screen:
  • change default shield
  • create new shield
  • info button

What is missing (future task):

  • status of shield ("action required", "applied and working")
  • status icons

How to test

  1. Create shields
  2. Check shields are presented in “Security Shields” screen.
  3. Change default shields and confirm the new default is set.

Video

here

PR submission checklist

  • I have tested “Security Shields” screen.

@giannis-rdx giannis-rdx self-assigned this Feb 17, 2025
@giannis-rdx giannis-rdx force-pushed the feature/ABW-4081-security-shields-list branch from 970972a to ed96778 Compare February 18, 2025 08:05
@giannis-rdx giannis-rdx requested review from sergiupuhalschi-rdx and micbakos-rdx and removed request for sergiupuhalschi-rdx February 18, 2025 08:41
@giannis-rdx giannis-rdx marked this pull request as ready for review February 18, 2025 08:42
Copy link
Contributor

@sergiupuhalschi-rdx sergiupuhalschi-rdx left a comment

Choose a reason for hiding this comment

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

Looks good

Comment on lines 96 to 97
_state.update { state -> state.copy(isChangingMainSecurityShieldInProgress = false) }
onDismissMainSecurityShieldBottomSheet()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that isChangingMainSecurityShieldInProgress = false could be inside onDismissMainSecurityShieldBottomSheet(). On that way you could save one less state update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 83 to 86
accountsCount = item.shieldForDisplay.numberOfLinkedAccounts.toInt(),
personasCount = item.shieldForDisplay.numberOfLinkedPersonas.toInt(),
hasAnyHiddenEntities = item.shieldForDisplay.numberOfLinkedHiddenAccounts.toInt() != 0 ||
item.shieldForDisplay.numberOfLinkedHiddenPersonas.toInt() != 0
Copy link
Contributor

Choose a reason for hiding this comment

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

You could create some computed properties in the SecurityShieldCard for all these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@micbakos-rdx micbakos-rdx left a comment

Choose a reason for hiding this comment

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

Good job!

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube Cloud

@giannis-rdx giannis-rdx merged commit 15e582e into main Feb 18, 2025
10 of 11 checks passed
@giannis-rdx giannis-rdx deleted the feature/ABW-4081-security-shields-list branch February 18, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants