Repository: go-co-op/gocron
Pull request #178 (2021-05-19 08:10:00)

Delete unique tag alongside RemoveByTag
### What does this do?
Currently, when you create jobs with unique tags and delete by tag afterwards, the unique tag is deleted. This leads to a unique tag constraint when adding a new job with the previous unique tag.

This PR solves this issue by deleting the unique tag alongside the `RemoveByTag` method.

### List any changes that modify/break current functionality
Delete unique tag in `RemoveByTag`

### Have you included tests for your changes?
yes

### Notes
Thanks for maintaining this library. We're planning to integrate it for a major feature and the tagging functionality is a game-changer :)


arjunmahishi: LGTM!

Patch:
diff --git a/scheduler.go b/scheduler.go
index 42b9378..f943abe 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -528,6 +528,10 @@ func (s *Scheduler) RemoveByTag(tag string) error {
 	if err != nil {
 		return err
 	}
+
+	// Remove unique tag when exists
+	delete(s.tags, tag)
+
 	for _, job := range jobs {
 		s.RemoveByReference(job)
 	}


Test patch:
diff --git a/scheduler_test.go b/scheduler_test.go
index b8de0ff..c0c7520 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -401,29 +401,51 @@ func TestScheduler_RemoveByReference(t *testing.T) {
 }
 
 func TestScheduler_RemoveByTag(t *testing.T) {
-	s := NewScheduler(time.UTC)
+	t.Run("non unique tags", func(t *testing.T) {
+		s := NewScheduler(time.UTC)
 
-	// Creating 2 Jobs with Unique tags
-	tag1 := "tag one"
-	tag2 := "tag two"
-	_, err := s.Every(1).Second().Tag(tag1).Do(taskWithParams, 1, "hello") // index 0
-	require.NoError(t, err)
-	_, err = s.Every(1).Second().Tag(tag2).Do(taskWithParams, 2, "world") // index 1
-	require.NoError(t, err)
+		// Creating 2 Jobs with different tags
+		tag1 := "tag one"
+		tag2 := "tag two"
+		_, err := s.Every(1).Second().Tag(tag1).Do(taskWithParams, 1, "hello") // index 0
+		require.NoError(t, err)
+		_, err = s.Every(1).Second().Tag(tag2).Do(taskWithParams, 2, "world") // index 1
+		require.NoError(t, err)
 
-	// check Jobs()[0] tags is equal with tag "tag one" (tag1)
-	assert.Equal(t, s.Jobs()[0].Tags()[0], tag1, "Job With Tag 'tag one' is removed from index 0")
+		// check Jobs()[0] tags is equal with tag "tag one" (tag1)
+		assert.Equal(t, s.Jobs()[0].Tags()[0], tag1, "Job With Tag 'tag one' is removed from index 0")
 
-	err = s.RemoveByTag("tag one")
-	require.NoError(t, err)
-	assert.Equal(t, 1, s.Len(), "Incorrect number of jobs after removing 1 job")
+		err = s.RemoveByTag("tag one")
+		require.NoError(t, err)
+		assert.Equal(t, 1, s.Len(), "Incorrect number of jobs after removing 1 job")
+
+		// check Jobs()[0] tags is equal with tag "tag two" (tag2) after removing "tag one"
+		assert.Equal(t, s.Jobs()[0].Tags()[0], tag2, "Job With Tag 'tag two' is removed from index 0")
 
-	// check Jobs()[0] tags is equal with tag "tag two" (tag2) after removing "tag one"
-	assert.Equal(t, s.Jobs()[0].Tags()[0], tag2, "Job With Tag 'tag two' is removed from index 0")
+		// Removing Non Existent Job with "tag one" because already removed above (will not removing any jobs because tag not match)
+		err = s.RemoveByTag("tag one")
+		assert.EqualError(t, err, ErrJobNotFoundWithTag.Error())
+	})
+
+	t.Run("unique tags", func(t *testing.T) {
+		s := NewScheduler(time.UTC)
+		s.TagsUnique()
 
-	// Removing Non Existent Job with "tag one" because already removed above (will not removing any jobs because tag not match)
-	err = s.RemoveByTag("tag one")
-	assert.EqualError(t, err, ErrJobNotFoundWithTag.Error())
+		// Creating 2 Jobs with unique tags
+		tag1 := "tag one"
+		tag2 := "tag two"
+		_, err := s.Every(1).Second().Tag(tag1).Do(taskWithParams, 1, "hello") // index 0
+		require.NoError(t, err)
+		_, err = s.Every(1).Second().Tag(tag2).Do(taskWithParams, 2, "world") // index 1
+		require.NoError(t, err)
+
+		err = s.RemoveByTag("tag one")
+		require.NoError(t, err)
+
+		// Adding job with tag after removing by tag, assuming the unique tag has been removed as well
+		_, err = s.Every(1).Second().Tag(tag1).Do(taskWithParams, 1, "hello")
+		assert.Nil(t, err, "Unique tag is not deleted when removing by tag")
+	})
 }
 
 func TestScheduler_Jobs(t *testing.T) {