pkg/files/error.go

@@ -0,0 +1,33 @@
+// Copyright 2019 Vikunja and contriubtors. All rights reserved.
+//
+// This file is part of Vikunja.
+//
+// Vikunja is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Vikunja is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Vikunja.  If not, see <https://www.gnu.org/licenses/>.
+
+package files
+
+import "fmt"
+
+type ErrFileDoesNotExist struct {
+	FileID int64
+}
+
+func (err ErrFileDoesNotExist) Error() string {
+	return fmt.Sprintf("file %d does not exist", err.FileID)
+}
+
+func IsErrFileDoesNotExist(err error) bool {
+	_, ok := err.(ErrFileDoesNotExist)
+	return ok
+}

pkg/files/files.go

@@ -19,7 +19,6 @@ package files
 import (
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/web"
-	"fmt"
 	"github.com/spf13/afero"
 	"io"
 	"strconv"
@@ -61,7 +60,7 @@ func (f *File) LoadFileByID() (err error) {
 func (f *File) LoadFileMetaByID() (err error) {
 	exists, err := x.Where("id = ?", f.ID).Get(f)
 	if !exists {
-		return fmt.Errorf("file %d does not exist", f.ID)
+		return ErrFileDoesNotExist{FileID: f.ID}
 	}
 	return
 }

pkg/models/task_attachment.go

@@ -20,6 +20,7 @@ import (
 	"code.vikunja.io/api/pkg/files"
 	"code.vikunja.io/web"
 	"io"
+	"os"
 )
 
 // TaskAttachment is the definition of a task attachment
@@ -119,7 +120,7 @@ func (ta *TaskAttachment) ReadAll(s string, a web.Auth, page int) (interface{},
 func (ta *TaskAttachment) Delete() error {
 	// Load the attachment
 	err := ta.ReadOne()
-	if err != nil {
+	if err != nil && !files.IsErrFileDoesNotExist(err) {
 		return err
 	}
 
@@ -130,5 +131,10 @@ func (ta *TaskAttachment) Delete() error {
 	}
 
 	// Delete the underlying file
-	return ta.File.Delete()
+	err = ta.File.Delete()
+	// If the file does not exist, we don't want to error out
+	if err != nil && os.IsNotExist(err) {
+		return nil
+	}
+	return err
 }

pkg/models/task_attachment_test.go

@@ -120,9 +120,22 @@ func TestTaskAttachment_ReadAll(t *testing.T) {
 
 func TestTaskAttachment_Delete(t *testing.T) {
 	t.Run("Normal", func(t *testing.T) {
-		// TODO: also check the actual file is being deleted
+		ta := &TaskAttachment{ID: 1}
+		err := ta.Delete()
+		assert.NoError(t, err)
+		// Check if the file itself was deleted
+		_, err = files.FileStat("/1") // The new file has the id 2 since it's the second attachment
+		assert.True(t, os.IsNotExist(err))
 	})
 	t.Run("Nonexisting", func(t *testing.T) {
-
+		ta := &TaskAttachment{ID: 9999}
+		err := ta.Delete()
+		assert.Error(t, err)
+		assert.True(t, IsErrTaskAttachmentDoesNotExist(err))
+	})
+	t.Run("Existing attachment, nonexisting file", func(t *testing.T) {
+		ta := &TaskAttachment{ID: 2}
+		err := ta.Delete()
+		assert.NoError(t, err)
 	})
 }