Make action name a required field

Fixes #66

This is a BREAKING CHANGE!

We've found that specifying action name is best practice because it makes it easier to relate a failure back to which action failed.
This makes naming an action no longer optional.

Author: rubberduck203

examples/group-1.yaml

@@ -8,7 +8,8 @@ spec:
   needs:
     - bar
   actions:
-    - description: foo1
+    - name: action1
+      description: foo1
       check:
         paths:
           - 'flig/bar/**/*'
@@ -21,7 +22,8 @@ spec:
           There is a good way to fix this, maybe...
         helpUrl: https://go.example.com/fixit
       required: false
-    - description: foo2
+    - name: action2
+      description: foo2
       check:
         paths:
           - '*/*.txt'

examples/v1alpha/DoctorGroup.yaml

@@ -8,7 +8,8 @@ spec:
   needs:
     - bar
   actions:
-    - description: foo1
+    - name: action1
+      description: foo1
       check:
         paths:
           - 'flig/bar/**/*'
@@ -21,7 +22,8 @@ spec:
           There is a good way to fix this, maybe...
         helpUrl: https://go.example.com/fixit
       required: false
-    - description: foo2
+    - name: action2
+      description: foo2
       check:
         paths:
           - '*/*.txt'

schema/merged.json

@@ -114,7 +114,8 @@
       "description": "An action is a single step used to check in a group. This is most commonly used to build a series of tasks for a system, like `ruby`, `python`, and databases.",
       "type": "object",
       "required": [
-        "check"
+        "check",
+        "name"
       ],
       "properties": {
         "check": {
@@ -142,12 +143,8 @@
           "nullable": true
         },
         "name": {
-          "description": "Name of the \"action\". When not provided, it will be the index of the action within the group. This is used when reporting status to the users.",
-          "type": [
-            "string",
-            "null"
-          ],
-          "nullable": true
+          "description": "Name of the \"action\" used when reporting status to the users.",
+          "type": "string"
         },
         "required": {
           "description": "If false, the action is allowed to fail and let other actions in the group execute. Defaults to `true`.",

schema/v1alpha.com.github.scope.ScopeDoctorGroup.json

@@ -130,7 +130,8 @@
       "description": "An action is a single step used to check in a group. This is most commonly used to build a series of tasks for a system, like `ruby`, `python`, and databases.",
       "type": "object",
       "required": [
-        "check"
+        "check",
+        "name"
       ],
       "properties": {
         "check": {
@@ -158,12 +159,8 @@
           "nullable": true
         },
         "name": {
-          "description": "Name of the \"action\". When not provided, it will be the index of the action within the group. This is used when reporting status to the users.",
-          "type": [
-            "string",
-            "null"
-          ],
-          "nullable": true
+          "description": "Name of the \"action\" used when reporting status to the users.",
+          "type": "string"
         },
         "required": {
           "description": "If false, the action is allowed to fail and let other actions in the group execute. Defaults to `true`.",

schema/v1alpha.com.github.scope.ScopeKnownError.json

@@ -130,7 +130,8 @@
       "description": "An action is a single step used to check in a group. This is most commonly used to build a series of tasks for a system, like `ruby`, `python`, and databases.",
       "type": "object",
       "required": [
-        "check"
+        "check",
+        "name"
       ],
       "properties": {
         "check": {
@@ -158,12 +159,8 @@
           "nullable": true
         },
         "name": {
-          "description": "Name of the \"action\". When not provided, it will be the index of the action within the group. This is used when reporting status to the users.",
-          "type": [
-            "string",
-            "null"
-          ],
-          "nullable": true
+          "description": "Name of the \"action\" used when reporting status to the users.",
+          "type": "string"
         },
         "required": {
           "description": "If false, the action is allowed to fail and let other actions in the group execute. Defaults to `true`.",

schema/v1alpha.com.github.scope.ScopeReportLocation.json

@@ -130,7 +130,8 @@
       "description": "An action is a single step used to check in a group. This is most commonly used to build a series of tasks for a system, like `ruby`, `python`, and databases.",
       "type": "object",
       "required": [
-        "check"
+        "check",
+        "name"
       ],
       "properties": {
         "check": {
@@ -158,12 +159,8 @@
           "nullable": true
         },
         "name": {
-          "description": "Name of the \"action\". When not provided, it will be the index of the action within the group. This is used when reporting status to the users.",
-          "type": [
-            "string",
-            "null"
-          ],
-          "nullable": true
+          "description": "Name of the \"action\" used when reporting status to the users.",
+          "type": "string"
         },
         "required": {
           "description": "If false, the action is allowed to fail and let other actions in the group execute. Defaults to `true`.",

src/models/v1alpha/doctor_group.rs

@@ -69,9 +69,8 @@ pub struct DoctorFixPromptSpec {
 #[serde(rename_all = "camelCase")]
 #[schemars(deny_unknown_fields)]
 pub struct DoctorGroupActionSpec {
-    /// Name of the "action". When not provided, it will be the index of the action within the group.
-    /// This is used when reporting status to the users.
-    pub name: Option<String>,
+    /// Name of the "action" used when reporting status to the users.
+    pub name: String,
 
     /// A description of this specific action, used for information to the users.
     pub description: Option<String>,

src/shared/models/internal/doctor_group.rs

@@ -75,8 +75,8 @@ impl TryFrom<V1AlphaDoctorGroup> for DoctorGroup {
 
     fn try_from(model: V1AlphaDoctorGroup) -> Result<Self, Self::Error> {
         let mut actions: Vec<_> = Default::default();
-        for (count, spec_action) in model.spec.actions.iter().enumerate() {
-            actions.push(parse_action(count, &model, spec_action)?);
+        for spec_action in model.spec.actions.iter() {
+            actions.push(parse_action(&model, spec_action)?);
         }
 
         Ok(DoctorGroup {
@@ -92,7 +92,6 @@ impl TryFrom<V1AlphaDoctorGroup> for DoctorGroup {
 }
 
 fn parse_action(
-    idx: usize,
     group_model: &V1AlphaDoctorGroup,
     action: &DoctorGroupActionSpec,
 ) -> Result<DoctorGroupAction> {
@@ -123,7 +122,7 @@ fn parse_action(
     };
 
     Ok(DoctorGroupAction {
-        name: spec_action.name.unwrap_or_else(|| format!("{}", idx + 1)),
+        name: spec_action.name,
         required: spec_action.required,
         description: spec_action
             .description
@@ -174,7 +173,7 @@ mod tests {
         assert_eq!(
             dg.actions[0],
             DoctorGroupAction {
-                name: "1".to_string(),
+                name: "action1".to_string(),
                 required: false,
                 description: "foo1".to_string(),
                 fix: DoctorFix {
@@ -195,7 +194,7 @@ mod tests {
         assert_eq!(
             dg.actions[1],
             DoctorGroupAction {
-                name: "2".to_string(),
+                name: "action2".to_string(),
                 required: true,
                 description: "foo2".to_string(),
                 fix: DoctorFix {

tests/scope_doctor.rs

@@ -100,7 +100,7 @@ fn test_cache_invalidation() {
     result
         .success()
         .stdout(predicate::str::contains(
-            "Check initially failed, fix was successful, group: \"setup\", name: \"1\"",
+            "Check initially failed, fix was successful, group: \"setup\", name: \"install\"",
         ))
         .stdout(predicate::str::contains("Failed to write updated cache to disk").not());
 
@@ -109,7 +109,7 @@ fn test_cache_invalidation() {
     result
         .success()
         .stdout(predicate::str::contains(
-            "Check was successful, group: \"setup\", name: \"1\"",
+            "Check was successful, group: \"setup\", name: \"install\"",
         ))
         .stdout(predicate::str::contains("Failed to write updated cache to disk").not());
 
@@ -124,7 +124,7 @@ fn test_cache_invalidation() {
     result
         .success()
         .stdout(predicate::str::contains(
-            "Check initially failed, fix was successful, group: \"setup\", name: \"1\"",
+            "Check initially failed, fix was successful, group: \"setup\", name: \"install\"",
         ))
         .stdout(predicate::str::contains("Failed to write updated cache to disk").not());
 }

tests/test-cases/file-cache-check/.scope/group.yaml

@@ -5,7 +5,8 @@ metadata:
   description: Run dep install
 spec:
   actions:
-    - check:
+    - name: install
+      check:
         paths:
           - '**/requirements.txt'
       fix: