Skip to content

[FIL-758] Allow to close ane reopen application #269

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

Merged
merged 4 commits into from
Apr 24, 2025

Conversation

Filip-L
Copy link
Collaborator

@Filip-L Filip-L commented Apr 22, 2025

No description provided.

@Filip-L Filip-L requested a review from kacperzuk-neti April 22, 2025 11:55
@Filip-L Filip-L temporarily deployed to production-fidl April 22, 2025 11:59 — with GitHub Actions Inactive
.service(router::application::allocation_failed),
.service(router::application::allocation_failed)
.service(router::application::decline)
.service(router::application::reopen_decline_application),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.service(router::application::reopen_decline_application),
.service(router::application::reopen_declined_application),

Copy link
Collaborator

Choose a reason for hiding this comment

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

Applies to all reopen_decline_application functions

@@ -272,6 +272,7 @@ pub enum AppState {
StartSignDatacap,
Granted,
TotalDatacapReached,
DeclineApplication,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
DeclineApplication,
Declined,

Decline is a verb, we want an adjective here.
No need to add Application in variant name, when this entire type already is called AppState, so it's obvious it's about an application

@@ -14,6 +14,7 @@ impl AppState {
AppState::StartSignDatacap => "start sign datacap",
AppState::Granted => "granted",
AppState::TotalDatacapReached => "total datacap reached",
AppState::DeclineApplication => "decline application",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
AppState::DeclineApplication => "decline application",
AppState::DeclineApplication => "declined",

Comment on lines 120 to 122
pub fn close_application(self) -> Self {
LifeCycle {
state: AppState::DeclineApplication,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Declined specifically means that someone reviewed the application and rejected it on some grounds. Closing application is a more generic term, that could also apply in cases like client withdrawing the application or reaching the total requested datacap.
In fact, close_application is called when total datacap is reached - setting the state to Declined in such case is very confusing. It should be a separate state (or even just leave it as Granted, just mark as inactive?)

@Filip-L Filip-L temporarily deployed to production-fidl April 23, 2025 11:38 — with GitHub Actions Inactive
@Filip-L Filip-L force-pushed the FIL-758-allow-to-close-and-reopen-application branch from 3eeb8ad to c9076c8 Compare April 23, 2025 15:50
@Filip-L Filip-L temporarily deployed to production-fidl April 23, 2025 15:52 — with GitHub Actions Inactive
@Filip-L Filip-L requested a review from kacperzuk-neti April 23, 2025 15:58
@Filip-L Filip-L merged commit b02b039 into main Apr 24, 2025
4 checks passed
@Filip-L Filip-L deleted the FIL-758-allow-to-close-and-reopen-application branch April 24, 2025 14:55
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