-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[PM-12403] - Implement Remove Send policy on Add/edit screen #11178
Conversation
No New Or Fixed Issues Found |
This reverts commit 832564d.
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #11178 +/- ##
==========================================
+ Coverage 35.05% 35.17% +0.12%
==========================================
Files 2711 2718 +7
Lines 84576 84911 +335
Branches 16069 16161 +92
==========================================
+ Hits 29649 29870 +221
- Misses 53956 54063 +107
- Partials 971 978 +7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of pulling in the policyService and checking if DisableSend is active, you can grab that information from the SendFormConfig(areSendsAllowed
), which is required and passed in from the hosting component. This should simplify the component and the pattern applies to all components modified in this PR.
2909c26
to
281d82d
Compare
This reverts commit 832564d.
🤦 Not sure how I missed that one! That definitely simplifies things. |
export class SendOptionsComponent implements OnInit, OnDestroy { | ||
private destroy$ = new Subject<void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OnDestroy
and destroy$
aren't used anywhere and can be removed. This was previously introduced for subscribing to policy changes, but could have also been done with takeUntilDestroyed()
@@ -27,6 +29,8 @@ import { SendFormContainer } from "../../send-form-container"; | |||
], | |||
}) | |||
export class SendFileDetailsComponent implements OnInit { | |||
private destroy$ = new Subject<void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -40,6 +44,7 @@ export class SendFileDetailsComponent implements OnInit { | |||
constructor( | |||
private formBuilder: FormBuilder, | |||
protected sendFormContainer: SendFormContainer, | |||
private policyService: PolicyService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
policyService
is no longer necessary
export class SendDetailsComponent implements OnInit, OnDestroy { | ||
private destroy$ = new Subject<void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OnDestroy
and destroy$
aren't used anywhere and can be removed. This was previously introduced for subscribing to policy changes, but could have also been done with takeUntilDestroyed()
|
||
constructor( | ||
protected sendFormContainer: SendFormContainer, | ||
protected formBuilder: FormBuilder, | ||
protected i18nService: I18nService, | ||
protected datePipe: DatePipe, | ||
protected environmentService: EnvironmentService, | ||
private policyService: PolicyService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
policyService
is no longer necessary
export class SendTextDetailsComponent implements OnInit, OnDestroy { | ||
private destroy$ = new Subject<void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OnDestroy
and destroy$
aren't used anywhere and can be removed. This was previously introduced for subscribing to policy changes, but could have also been done with takeUntilDestroyed()
@@ -35,6 +39,7 @@ export class SendTextDetailsComponent implements OnInit { | |||
constructor( | |||
private formBuilder: FormBuilder, | |||
protected sendFormContainer: SendFormContainer, | |||
private policyService: PolicyService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
policyService
is no longer necessary
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-12410
📔 Objective
This PR adds logic to disable all of the send forms if the policy requires. I also merged the base send details and send details components as it is not necessary or helpful to keep them separate.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes