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: allow running a legal hold manually #129

Open
wants to merge 50 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
8f2acdd
refactor: Split `run` method into `run` and `runWith` with legal hold…
fmartingr Jan 8, 2025
7c431e3
feat: Add mutex protection for legal hold execution to prevent concur…
fmartingr Jan 8, 2025
df241a8
feat: Add endpoint and method to manually run a single legal hold job
fmartingr Jan 8, 2025
5a6b02c
feat: Add manual run action for legal hold with confirmation and erro…
fmartingr Jan 8, 2025
45c4dfa
feat: Add last execution time display in legal hold table
fmartingr Jan 8, 2025
3a8d858
style: Adjust margin of "Run legal hold" button to match other buttons
fmartingr Jan 8, 2025
6bd2408
feat: Add LastMessageAt attribute to track latest message in LegalHold
fmartingr Jan 8, 2025
d4232fd
fix: Add missing fields to LegalHold TypeScript interface
fmartingr Jan 8, 2025
28a84f8
fix: use LockWithContext to get locking errors rather than queuing
fmartingr Jan 9, 2025
a20034d
feat: Add Status field to LegalHold with idle and executing states
fmartingr Jan 9, 2025
14b12db
feat: Add refresh button to legal holds table header
fmartingr Jan 9, 2025
d9fbf7d
feat: Update legal hold execution to track and update status
fmartingr Jan 9, 2025
544087d
refactor: Add helper method to update legal hold status atomically
fmartingr Jan 9, 2025
d0f56fe
feat: Add UpdateLegalHoldStatus method to legal hold implementation
fmartingr Jan 9, 2025
ee1d0dd
test: Add comprehensive tests for UpdateLegalHoldStatus method
fmartingr Jan 9, 2025
21369eb
refactor: Update legal hold status method signature to return only error
fmartingr Jan 13, 2025
1cb2ed8
refactor: Update Execute method to return updated LegalHold instead o…
fmartingr Jan 13, 2025
010c082
feat: Disable download and run buttons based on legal hold status
fmartingr Jan 13, 2025
2ded0ba
refactor: Simplify run confirmation modal trigger logic
fmartingr Jan 13, 2025
525aabe
feat: Show "Running..." for legal hold when status is executing
fmartingr Jan 13, 2025
adffbf1
refactor: Extract display logic for last run and message into separat…
fmartingr Jan 13, 2025
a698f0d
fix: ensure updates to legal hold attributes correctly
fmartingr Jan 13, 2025
616219a
fix: execution global timeout type
fmartingr Jan 13, 2025
54b2c6c
feat: Add refresh functionality after running legal hold
fmartingr Jan 13, 2025
ba0a5cf
fix: Resolve golangci-lint shadowing warnings in legal_hold.go
fmartingr Jan 13, 2025
cf06433
fix: disallow run button if legal hold is executing
fmartingr Jan 13, 2025
7e2363b
fix: force condition
fmartingr Jan 13, 2025
49746ff
fix: avoid double loop, continue with next legal holds on error
fmartingr Jan 13, 2025
646435b
feat: Add endpoint to reset legal hold status to IDLE
fmartingr Jan 13, 2025
7853d55
refactor: Remove "Last Message" column from legal hold table
fmartingr Jan 13, 2025
52f4a2a
Merge remote-tracking branch 'origin/main' into feat/run-manually
fmartingr Jan 13, 2025
423d374
test: Add API tests for resetstatus and run legal hold endpoints
fmartingr Jan 13, 2025
cfadfd6
fix(aider): some messups in the code
fmartingr Jan 13, 2025
adfafa3
fix: styles
fmartingr Jan 13, 2025
3c89687
chore: fixed comment
fmartingr Jan 14, 2025
4967cb5
feat: Add confirmation modal for legal hold reset action
fmartingr Jan 14, 2025
057ea82
refactor: Remove click-to-reset functionality from legal hold row
fmartingr Jan 15, 2025
3388964
feat: Reset all legal hold statuses to pending on plugin activation
fmartingr Jan 15, 2025
88ec7c7
chore: added TODO comments for OverlayTrigger
fmartingr Jan 15, 2025
35baec9
feat: Add last execution timestamp to legal hold row display
fmartingr Jan 15, 2025
26c284d
fix: Convert legal hold timestamp to milliseconds for correct date di…
fmartingr Jan 15, 2025
d11eb39
chore: change system admin comment
fmartingr Jan 15, 2025
206e39c
refactor: Remove resetstatus endpoint and related code
fmartingr Jan 15, 2025
2daf648
test: updated to follow changes in @e2e-support
fmartingr Jan 16, 2025
9653336
chore: reset colors to use theme variables
fmartingr Jan 20, 2025
aacffe6
fix: do not block plugin activation on legal hold reset
fmartingr Jan 20, 2025
f3065d4
fix: ensure defaults before update kvstore
fmartingr Jan 20, 2025
21b6b41
fix: omitempty for new fields
fmartingr Jan 20, 2025
605baa5
fix: use different field type to store legal hold download state
fmartingr Jan 21, 2025
07827f9
chore: typo, better text
fmartingr Jan 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ func (p *Plugin) ServeHTTP(_ *plugin.Context, w http.ResponseWriter, r *http.Req
router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}/release", p.releaseLegalHold).Methods(http.MethodPost)
router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}", p.updateLegalHold).Methods(http.MethodPut)
router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}/download", p.downloadLegalHold).Methods(http.MethodGet)
router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}/run", p.runSingleLegalHold).Methods(http.MethodPost)
router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}/resetstatus", p.resetLegalHoldStatus).Methods(http.MethodPost)
router.HandleFunc("/api/v1/test_amazon_s3_connection", p.testAmazonS3Connection).Methods(http.MethodPost)

// Other routes
Expand Down Expand Up @@ -320,6 +322,71 @@ func (p *Plugin) runJobFromAPI(w http.ResponseWriter, _ *http.Request) {
go p.legalHoldJob.RunFromAPI()
}

func (p *Plugin) runSingleLegalHold(w http.ResponseWriter, r *http.Request) {
legalholdID, err := RequireLegalHoldID(r)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

err = p.legalHoldJob.RunSingleLegalHold(legalholdID)
if err != nil {
http.Error(w, fmt.Sprintf("Failed to run legal hold: %s", err.Error()), http.StatusInternalServerError)
return
}

response := struct {
Message string `json:"message"`
}{
Message: fmt.Sprintf("Processing Legal Hold %s. Please check the MM server logs for more details.", legalholdID),
}

b, err := json.Marshal(response)
if err != nil {
http.Error(w, "Error encoding json", http.StatusInternalServerError)
return
}

w.Header().Set("Content-Type", "application/json")
wiggin77 marked this conversation as resolved.
Show resolved Hide resolved
_, err = w.Write(b)
if err != nil {
p.API.LogError("failed to write http response", err.Error())
}
}

// resetLegalHoldStatus resets the status of a LegalHold to Idle
func (p *Plugin) resetLegalHoldStatus(w http.ResponseWriter, r *http.Request) {
fmartingr marked this conversation as resolved.
Show resolved Hide resolved
legalholdID, err := RequireLegalHoldID(r)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

err = p.KVStore.UpdateLegalHoldStatus(legalholdID, model.LegalHoldStatusIdle)
if err != nil {
http.Error(w, fmt.Sprintf("Failed to reset legal hold status: %s", err.Error()), http.StatusInternalServerError)
return
}

response := struct {
Message string `json:"message"`
}{
Message: fmt.Sprintf("Successfully reset status for Legal Hold %s", legalholdID),
}

b, err := json.Marshal(response)
if err != nil {
http.Error(w, "Error encoding json", http.StatusInternalServerError)
return
}

w.Header().Set("Content-Type", "application/json")
_, err = w.Write(b)
if err != nil {
p.API.LogError("failed to write http response", err.Error())
}
}

// testAmazonS3Connection tests the plugin's custom Amazon S3 connection
func (p *Plugin) testAmazonS3Connection(w http.ResponseWriter, _ *http.Request) {
type messageResponse struct {
Expand Down
134 changes: 134 additions & 0 deletions server/api_test.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,152 @@
package main

import (
"fmt"
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/gorilla/mux"
pluginapi "github.com/mattermost/mattermost-plugin-api"
"github.com/mattermost/mattermost-plugin-legal-hold/server/config"
"github.com/mattermost/mattermost-plugin-legal-hold/server/store/kvstore"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/plugin/plugintest"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

type MockLegalHoldJob struct {
mock.Mock
}

func (m *MockLegalHoldJob) GetID() string {
args := m.Called()
return args.String(0)
}

func (m *MockLegalHoldJob) OnConfigurationChange(cfg *config.Configuration) error {
args := m.Called(cfg)
return args.Error(0)
}

func (m *MockLegalHoldJob) Stop(timeout time.Duration) error {
args := m.Called(timeout)
return args.Error(0)
}

func (m *MockLegalHoldJob) RunFromAPI() {
m.Called()
}

func (m *MockLegalHoldJob) RunSingleLegalHold(id string) error {
args := m.Called(id)
return args.Error(0)
}

func TestResetLegalHoldStatus(t *testing.T) {
p := &Plugin{}
api := &plugintest.API{}
p.SetDriver(&plugintest.Driver{})
p.SetAPI(api)
p.Client = pluginapi.NewClient(p.API, p.Driver)

// Initialize KVStore
p.KVStore = kvstore.NewKVStore(p.Client)

// Initialize the router
p.router = mux.NewRouter()
p.router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}/resetstatus", p.resetLegalHoldStatus).Methods(http.MethodPost)
p.router.HandleFunc("/api/v1/legalholds/{legalhold_id:[A-Za-z0-9]+}/run", p.runSingleLegalHold).Methods(http.MethodPost)

api.On("HasPermissionTo", "test_user_id", model.PermissionManageSystem).Return(true)
api.On("LogInfo", mock.Anything).Maybe()
api.On("LogError", mock.Anything, mock.Anything).Maybe()

// Test successful reset
testID := model.NewId()
api.On("KVGet", mock.AnythingOfType("string")).Return([]byte(fmt.Sprintf(`{"id":"%s","status":"executing"}`, testID)), nil).Once()
api.On("KVSetWithOptions", mock.AnythingOfType("string"), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("model.PluginKVSetOptions")).Return(true, nil).Once()

req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("/api/v1/legalholds/%s/resetstatus", testID), nil)
require.NoError(t, err)
req.Header.Add("Mattermost-User-Id", "test_user_id")

recorder := httptest.NewRecorder()
p.ServeHTTP(nil, recorder, req)
require.Equal(t, http.StatusOK, recorder.Code)

// Test invalid legal hold ID
req, err = http.NewRequest(http.MethodPost, "/api/v1/legalholds/invalid_id/resetstatus", nil)
require.NoError(t, err)
req.Header.Add("Mattermost-User-Id", "test_user_id")

recorder = httptest.NewRecorder()
p.ServeHTTP(nil, recorder, req)
require.Equal(t, http.StatusNotFound, recorder.Code)

// Test non-existent legal hold
api.On("KVGet", mock.AnythingOfType("string")).Return(nil, &model.AppError{}).Once()

req, err = http.NewRequest(http.MethodPost, "/api/v1/legalholds/"+model.NewId()+"/resetstatus", nil)
require.NoError(t, err)
req.Header.Add("Mattermost-User-Id", "test_user_id")

recorder = httptest.NewRecorder()
p.ServeHTTP(nil, recorder, req)
require.Equal(t, http.StatusInternalServerError, recorder.Code)
}

func TestRunSingleLegalHold(t *testing.T) {
p := &Plugin{}
api := &plugintest.API{}
p.SetDriver(&plugintest.Driver{})
p.SetAPI(api)
p.Client = pluginapi.NewClient(p.API, p.Driver)

api.On("HasPermissionTo", "test_user_id", model.PermissionManageSystem).Return(true)
api.On("LogInfo", mock.Anything).Maybe()
api.On("LogError", mock.Anything, mock.Anything).Maybe()

// Mock the legal hold job
mockJob := &MockLegalHoldJob{}
p.legalHoldJob = mockJob

// Test successful run
testID := model.NewId()
mockJob.On("RunSingleLegalHold", testID).Return(nil).Once()

req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("/api/v1/legalholds/%s/run", testID), nil)
require.NoError(t, err)
req.Header.Add("Mattermost-User-Id", "test_user_id")

recorder := httptest.NewRecorder()
p.ServeHTTP(nil, recorder, req)
require.Equal(t, http.StatusOK, recorder.Code)

// Test invalid legal hold ID
req, err = http.NewRequest(http.MethodPost, "/api/v1/legalholds/invalid_id/run", nil)
require.NoError(t, err)
req.Header.Add("Mattermost-User-Id", "test_user_id")

recorder = httptest.NewRecorder()
p.ServeHTTP(nil, recorder, req)
require.Equal(t, http.StatusNotFound, recorder.Code)

// Test error running legal hold
lhID := model.NewId()
mockJob.On("RunSingleLegalHold", lhID).Return(fmt.Errorf("test error")).Once()

req, err = http.NewRequest(http.MethodPost, "/api/v1/legalholds/"+lhID+"/run", nil)
require.NoError(t, err)
req.Header.Add("Mattermost-User-Id", "test_user_id")

recorder = httptest.NewRecorder()
p.ServeHTTP(nil, recorder, req)
require.Equal(t, http.StatusInternalServerError, recorder.Code)
}

func TestTestAmazonS3Connection(t *testing.T) {
p := &Plugin{}
api := &plugintest.API{}
Expand Down
104 changes: 68 additions & 36 deletions server/jobs/legal_hold_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"strings"
"sync"
"time"

Expand All @@ -16,6 +17,7 @@ import (

"github.com/mattermost/mattermost-plugin-legal-hold/server/config"
"github.com/mattermost/mattermost-plugin-legal-hold/server/legalhold"
"github.com/mattermost/mattermost-plugin-legal-hold/server/model"
"github.com/mattermost/mattermost-plugin-legal-hold/server/store/kvstore"
"github.com/mattermost/mattermost-plugin-legal-hold/server/store/sqlstore"
)
Expand Down Expand Up @@ -152,6 +154,20 @@ func (j *LegalHoldJob) RunFromAPI() {
j.run()
}

func (j *LegalHoldJob) RunSingleLegalHold(legalHoldID string) error {
// Retrieve the single legal hold from the store
legalHold, err := j.kvstore.GetLegalHoldByID(legalHoldID)
if err != nil {
return fmt.Errorf("failed to fetch legal hold: %w", err)
}
if legalHold == nil {
return fmt.Errorf("legal hold not found: %s", legalHoldID)
}

j.runWith([]model.LegalHold{*legalHold}, true)
return nil
}

func (j *LegalHoldJob) run() {
j.mux.Lock()
oldRunner := j.runner
Expand All @@ -162,6 +178,19 @@ func (j *LegalHoldJob) run() {
return
}

j.client.Log.Info("Processing all Legal Holds")

// Retrieve the legal holds from the store.
legalHolds, err := j.kvstore.GetAllLegalHolds()
if err != nil {
j.client.Log.Error("Failed to fetch legal holds from store", err)
return
}

j.runWith(legalHolds, false)
}

func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) {
j.client.Log.Info("Running Legal Hold Job")
exitSignal := make(chan struct{})
ctx, canceller := context.WithCancel(context.Background())
Expand All @@ -186,47 +215,50 @@ func (j *LegalHoldJob) run() {
settings = j.settings.Clone()
j.mux.Unlock()

// Retrieve the legal holds from the store.
legalHolds, err := j.kvstore.GetAllLegalHolds()
if err != nil {
j.client.Log.Error("Failed to fetch legal holds from store", err)
}
for _, lh := range legalHolds {
if lh.IsFinished() {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s has ended and therefore does not executing.", lh.ID))
continue
}

j.client.Log.Info("Processing all Legal Holds", "count", len(legalHolds))
now := mattermostModel.GetMillis()
if !forceRun && !lh.NeedsExecuting(now) {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s is not yet ready to be executed again.", lh.ID))
continue
}
if !forceRun && lh.LastExecutionEndedAt >= now {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s was already executed after the current time.", lh.ID))
continue
}

for _, lh := range legalHolds {
for {
if lh.IsFinished() {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s has ended and therefore will not execute.", lh.ID))
break
}
j.client.Log.Debug(fmt.Sprintf("Creating Legal Hold Execution for legal hold: %s", lh.ID))
lhe := legalhold.NewExecution(lh, j.papi, j.sqlstore, j.kvstore, j.filebackend)

if !lh.NeedsExecuting(mattermostModel.GetMillis()) {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s is not yet ready to be executed again.", lh.ID))
break
if updatedLH, err := lhe.Execute(); err != nil {
if strings.Contains(err.Error(), "another execution is already running") {
j.client.Log.Debug("Another execution is already running for this legal hold", "legal_hold_id", lh.ID)
continue
}

j.client.Log.Debug(fmt.Sprintf("Creating Legal Hold Execution for legal hold: %s", lh.ID))
lhe := legalhold.NewExecution(lh, j.papi, j.sqlstore, j.filebackend)

if end, err := lhe.Execute(); err != nil {
j.client.Log.Error("An error occurred executing the legal hold.", err)
} else {
old, err := j.kvstore.GetLegalHoldByID(lh.ID)
if err != nil {
j.client.Log.Error("Failed to fetch the LegalHold prior to updating", err)
continue
}
lh = *old
lh.LastExecutionEndedAt = end
newLH, err := j.kvstore.UpdateLegalHold(lh, *old)
if err != nil {
j.client.Log.Error("Failed to update legal hold", err)
continue
}
lh = *newLH
j.client.Log.Info("legal hold executed", "legal_hold_id", lh.ID, "legal_hold_name", lh.Name)
j.client.Log.Error("An error occurred executing the legal hold.", err)
} else {
// Update legal hold with the new execution details (last execution time and last message)
// Also set it to IDLE again since the execution has ended.
old, err := j.kvstore.GetLegalHoldByID(lh.ID)
if err != nil {
j.client.Log.Error("Failed to fetch the LegalHold prior to updating", err)
continue
}
lh = *old
lh.LastExecutionEndedAt = updatedLH.LastExecutionEndedAt
lh.LastMessageAt = updatedLH.LastMessageAt
lh.Status = model.LegalHoldStatusIdle
newLH, err := j.kvstore.UpdateLegalHold(lh, *old)
if err != nil {
j.client.Log.Error("Failed to update legal hold", err)
continue
}
lh = *newLH
j.client.Log.Info("legal hold executed", "legal_hold_id", lh.ID, "legal_hold_name", lh.Name)
}
}
_ = ctx
Expand Down
16 changes: 16 additions & 0 deletions server/jobs/legal_hold_job_interface.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package jobs

import (
"time"

"github.com/mattermost/mattermost-plugin-legal-hold/server/config"
)

// LegalHoldJobInterface defines the interface that both real and mock implementations must satisfy
type LegalHoldJobInterface interface {
GetID() string
OnConfigurationChange(cfg *config.Configuration) error
Stop(timeout time.Duration) error
RunFromAPI()
RunSingleLegalHold(id string) error
}
Loading
Loading