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

feat: add 'auto' approve feature to auth_cli #685

Merged
merged 9 commits into from
Oct 9, 2024

Conversation

gkc
Copy link
Contributor

@gkc gkc commented Oct 7, 2024

- What I did

  • feat: add --auto option to the approve command - requires an explicit number of matching enrolments to auto-approve; must be combined with the --arx and/or --drx options
  • refactor: deprecate the old src/activate_cli/activate_cli.dart as it has been superseded by src/cli/auth_cli.dart
  • upgraded at_cli_commons dependency to ^1.2.0

- How I did it
See commits

- How to verify it
Tests pass

gkc added 5 commits October 7, 2024 13:09
…has been superseded by src/cli/auth_cli.dart
Also:
- fix: standardized the AtClient storage path used in the auth_cli
- refactor: deprecate the old src/activate_cli/activate_cli.dart as it has been superseded by src/cli/auth_cli.dart
@gkc gkc requested a review from sitaram-kalluri October 8, 2024 18:49
@gkc gkc marked this pull request as ready for review October 9, 2024 08:07
@gkc
Copy link
Contributor Author

gkc commented Oct 9, 2024

#687 has been merged and published and the at_cli_commons dependency has been upgraded to ^1.2.0

@@ -6,11 +6,13 @@ import 'package:at_onboarding_cli/at_onboarding_cli.dart';
import 'package:at_onboarding_cli/src/util/at_onboarding_exceptions.dart';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file is deprecated as all functionality is now in cli/auth_cli.dart

@@ -20,10 +21,25 @@ final AtSignLogger logger = AtSignLogger(' CLI ');

final aca = AuthCliArgs();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now create ephemeral storage each time; we clean it up when we exit

}
}

Future<int> _main(List<String> arguments) async {
Future<int> wrappedMain(List<String> arguments) async {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

made public so that it can be called by the at_onboarding_cli_test in at_onboarding_cli_functional_tests

String nameSpace = 'at_auth_cli';
if (storageDir != null) {
throw StateError('AtClient has already been created');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a check to ensure we don't have a logic error elsewhere

progName: nameSpace,
uniqueID: '${DateTime.now().millisecondsSinceEpoch}',
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the utility function newly added to at_cli_commons 1.2.0

@@ -650,7 +684,8 @@ Future<Map> _fetchOrListAndFilter(
return enrollmentMap;
}

Future<void> approve(ArgResults ar, AtClient atClient) async {
Future<int> approve(ArgResults ar, AtClient atClient, {int? limit}) async {
int approved = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an optional limit parameter and track and return an approved count so that we can also call this function from the autoApprove function when required


if (limit != null && approved >= limit) {
return approved;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we were passed a limit parameter (e.g. by the autoApprove function) then let's respect it

return approved;
}

Future<int> autoApprove(ArgResults ar, AtClient atClient) async {
Copy link
Contributor Author

@gkc gkc Oct 9, 2024

Choose a reason for hiding this comment

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

New feature.

  • if requested, calls approve to approve any existing matching enrollment requests, and respecting the approval count limit while doing so
  • listen for new enrollment request notifications
  • when one is received, and it matches the supplied regex filter(s), then
    • approve it
    • check the approved count vs limit
    • if limit has been reached
      • complete the completer so the await completer.future completes
      • clean up the subscription

'-c',
params['cramkey']!,
],
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as mentioned earlier, activate_cli.dart has been deprecated as it has been entirely replaced by auth_cli.dart

@gkc gkc merged commit 22e6bfc into trunk Oct 9, 2024
11 checks passed
@gkc gkc deleted the feat-auth-cli-add-auto-approve branch October 9, 2024 10:33
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