From 8f2acdd40d7bac452ad0271b1e0663b24cc23862 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 8 Jan 2025 10:44:26 +0100 Subject: [PATCH 01/49] refactor: Split `run` method into `run` and `runWith` with legal hold execution control --- server/jobs/legal_hold_job.go | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index a97de27..353de27 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -16,6 +16,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" ) @@ -162,6 +163,17 @@ func (j *LegalHoldJob) run() { return } + // 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()) @@ -188,12 +200,6 @@ func (j *LegalHoldJob) run() { 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) - } - for _, lh := range legalHolds { for { if lh.IsFinished() { @@ -201,10 +207,15 @@ func (j *LegalHoldJob) run() { break } - if !lh.NeedsExecuting(mattermostModel.GetMillis()) { + 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)) break } + if forceRun && lh.LastExecutionEndedAt >= now { + j.client.Log.Debug(fmt.Sprintf("Legal Hold %s was already executed after the current time.", 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.filebackend) From 7c431e3ba76383949db64037d4a6dcaa5f10029e Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 8 Jan 2025 10:55:45 +0100 Subject: [PATCH 02/49] feat: Add mutex protection for legal hold execution to prevent concurrent runs --- server/jobs/legal_hold_job.go | 5 +++++ server/legalhold/legal_hold.go | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 353de27..35d357b 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "strings" "sync" "time" @@ -221,6 +222,10 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { lhe := legalhold.NewExecution(lh, j.papi, j.sqlstore, j.filebackend) if end, 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.Error("An error occurred executing the legal hold.", err) } else { old, err := j.kvstore.GetLegalHoldByID(lh.ID) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 4f2e596..048ec19 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/gocarina/gocsv" + "github.com/mattermost/mattermost-plugin-api/cluster" "github.com/mattermost/mattermost-server/v6/plugin" "github.com/mattermost/mattermost-server/v6/shared/filestore" @@ -55,7 +56,17 @@ func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQ // Execute executes the Execution. func (ex *Execution) Execute() (int64, error) { - err := ex.GetChannels() + // Lock multiple executions using a cluster mutex + mutexKey := fmt.Sprintf("legal_hold_%s_execution", ex.LegalHold.ID) + mutex, err := cluster.NewMutex(ex.papi, mutexKey) + if err != nil { + return 0, fmt.Errorf("failed to create cluster mutex: %w", err) + } + + mutex.Lock() + defer mutex.Unlock() + + err = ex.GetChannels() if err != nil { return 0, err } From df241a855926297996bd32dbfeb11358c94f822a Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 8 Jan 2025 11:10:34 +0100 Subject: [PATCH 03/49] feat: Add endpoint and method to manually run a single legal hold job --- server/api.go | 33 +++++++++++++++++++++++++++++++++ server/jobs/legal_hold_job.go | 14 ++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/server/api.go b/server/api.go index 62e6d76..e724352 100644 --- a/server/api.go +++ b/server/api.go @@ -44,6 +44,7 @@ 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/test_amazon_s3_connection", p.testAmazonS3Connection).Methods(http.MethodPost) // Other routes @@ -320,6 +321,38 @@ 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") + _, 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 { diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 35d357b..4cfd80d 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -154,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 From 5a6b02c700fc9f40694e1ddebd5a0f249b79f858 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 8 Jan 2025 11:51:28 +0100 Subject: [PATCH 04/49] feat: Add manual run action for legal hold with confirmation and error handling --- webapp/src/client.ts | 5 ++ .../legal_hold_row/legal_hold_row.tsx | 57 ++++++++++++++++++- .../legal_hold_row/play-outline.svg | 3 + .../legal_hold_row/run_confirmation_modal.tsx | 43 ++++++++++++++ .../legal_hold_row/run_error_modal.tsx | 35 ++++++++++++ .../legal_hold_table/legal_hold_table.tsx | 2 + webapp/src/components/legal_holds_setting.tsx | 11 ++++ 7 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 webapp/src/components/legal_hold_table/legal_hold_row/play-outline.svg create mode 100644 webapp/src/components/legal_hold_table/legal_hold_row/run_confirmation_modal.tsx create mode 100644 webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx diff --git a/webapp/src/client.ts b/webapp/src/client.ts index 0b0e9b9..1c8d141 100644 --- a/webapp/src/client.ts +++ b/webapp/src/client.ts @@ -32,6 +32,11 @@ class APIClient { return this.doWithBody(url, 'put', data); }; + runLegalHold = (id: string) => { + const url = `${this.url}/legalholds/${id}/run`; + return this.doWithBody(url, 'post', {}); + }; + testAmazonS3Connection = () => { const url = `${this.url}/test_amazon_s3_connection`; return this.doWithBody(url, 'post', {}) as Promise<{message: string}>; diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 1836ab3..3e722b2 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -1,8 +1,7 @@ -import React from 'react'; +import React, {useState} from 'react'; import {UserProfile} from 'mattermost-redux/types/users'; import {LegalHold} from '@/types'; -import Client from '@/client'; import Tooltip from '@/components/mattermost-webapp/tooltip'; @@ -11,6 +10,9 @@ import OverlayTrigger from '@/components/mattermost-webapp/overlay_trigger'; import DownloadIcon from './download-outline_F0B8F.svg'; import EditIcon from './pencil-outline_F0CB6.svg'; import EyeLockIcon from './eye-outline_F06D0.svg'; +import RunIcon from './play-outline.svg'; +import RunConfirmationModal from './run_confirmation_modal'; +import RunErrorModal from './run_error_modal'; interface LegalHoldRowProps { legalHold: LegalHold; @@ -18,9 +20,12 @@ interface LegalHoldRowProps { releaseLegalHold: Function; showUpdateModal: Function; showSecretModal: Function; + runLegalHold: (id: string) => Promise; } const LegalHoldRow = (props: LegalHoldRowProps) => { + const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); + const [showRunErrorModal, setShowRunErrorModal] = useState(false); const lh = props.legalHold; const startsAt = (new Date(lh.starts_at)).toLocaleDateString(); const endsAt = lh.ends_at === 0 ? 'Never' : (new Date(lh.ends_at)).toLocaleDateString(); @@ -29,7 +34,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { props.releaseLegalHold(lh); }; - const downloadUrl = Client.downloadUrl(lh.id); + const downloadUrl = `/plugins/com.mattermost.plugin-legal-hold/api/v1/legalholds/${lh.id}/download`; return ( @@ -136,6 +141,36 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { + + {'Run Legal Hold Now'} + + )} + > + setShowRunConfirmModal(true)} + style={{ + marginRight: '20px', + height: '24px', + }} + > + + + + + { className={'btn btn-danger'} >{'Release'} + setShowRunConfirmModal(false)} + onConfirm={() => { + setShowRunConfirmModal(false); + props.runLegalHold(lh.id).then(() => { + // Success - do nothing as the message from the API is enough + }).catch(() => { + setShowRunErrorModal(true); + }); + }} + /> + setShowRunErrorModal(false)} + /> ); }; diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/play-outline.svg b/webapp/src/components/legal_hold_table/legal_hold_row/play-outline.svg new file mode 100644 index 0000000..d1d9d91 --- /dev/null +++ b/webapp/src/components/legal_hold_table/legal_hold_row/play-outline.svg @@ -0,0 +1,3 @@ + + + diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/run_confirmation_modal.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/run_confirmation_modal.tsx new file mode 100644 index 0000000..4c48d8f --- /dev/null +++ b/webapp/src/components/legal_hold_table/legal_hold_row/run_confirmation_modal.tsx @@ -0,0 +1,43 @@ +import React from 'react'; +import {Modal} from 'react-bootstrap'; + +interface Props { + onHide: () => void; + onConfirm: () => void; + show: boolean; +} + +const RunConfirmationModal = ({show, onHide, onConfirm}: Props) => { + return ( + + + {'Confirm Legal Hold Run'} + + + {'Are you sure you want to run this legal hold now?'} + + + + + + + ); +}; + +export default RunConfirmationModal; diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx new file mode 100644 index 0000000..5b90026 --- /dev/null +++ b/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx @@ -0,0 +1,35 @@ +import React from 'react'; +import {Modal} from 'react-bootstrap'; + +interface Props { + onHide: () => void; + show: boolean; +} + +const RunErrorModal = ({show, onHide}: Props) => { + return ( + + + {'Error Running Legal Hold'} + + + {'There was an error running the legal hold. Please contact your system administrator.'} + + + + + + ); +}; + +export default RunErrorModal; diff --git a/webapp/src/components/legal_hold_table/legal_hold_table.tsx b/webapp/src/components/legal_hold_table/legal_hold_table.tsx index ceecc09..6677ef0 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_table.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_table.tsx @@ -11,6 +11,7 @@ interface LegalHoldTableProps { releaseLegalHold: Function, showUpdateModal: Function, showSecretModal: Function, + runLegalHold: (id: string) => Promise; } const LegalHoldTable = (props: LegalHoldTableProps) => { @@ -68,6 +69,7 @@ const LegalHoldTable = (props: LegalHoldTableProps) => { releaseLegalHold={props.releaseLegalHold} showUpdateModal={props.showUpdateModal} showSecretModal={props.showSecretModal} + runLegalHold={(id: string) => props.runLegalHold(id)} /> ); })} diff --git a/webapp/src/components/legal_holds_setting.tsx b/webapp/src/components/legal_holds_setting.tsx index da487e5..b0ef064 100644 --- a/webapp/src/components/legal_holds_setting.tsx +++ b/webapp/src/components/legal_holds_setting.tsx @@ -24,6 +24,16 @@ const LegalHoldsSetting = () => { const [showSecretModal, setShowSecretModal] = useState(false); const [activeLegalHold, setActiveLegalHold] = useState(null); + const doRunLegalHold = async (id: string) => { + try { + const response = await Client.runLegalHold(id); + return response; + } catch (error) { + console.log(error); //eslint-disable-line no-console + throw error; + } + }; + const createLegalHold = async (data: CreateLegalHold) => { try { const response = await Client.createLegalHold(data); @@ -173,6 +183,7 @@ const LegalHoldsSetting = () => { releaseLegalHold={doShowReleaseModal} showUpdateModal={doShowUpdateModal} showSecretModal={doShowSecretModal} + runLegalHold={doRunLegalHold} /> )} From 45c4dfa01d9e3194545a73aae6f6e95b96aa50d3 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 8 Jan 2025 15:26:42 +0100 Subject: [PATCH 05/49] feat: Add last execution time display in legal hold table --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 3 +++ webapp/src/components/legal_hold_table/legal_hold_table.tsx | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 3e722b2..f0f7dd5 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -45,6 +45,9 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{startsAt}
{endsAt}
{props.users.length} {'users'}
+
+ {lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'} +
{ aria-label='Legal Holds Table' style={{ display: 'grid', - gridTemplateColumns: 'auto auto auto auto auto', + gridTemplateColumns: 'auto auto auto auto auto auto', columnGap: '10px', rowGap: '10px', alignItems: 'center', @@ -57,6 +57,10 @@ const LegalHoldTable = (props: LegalHoldTableProps) => { aria-label='users header' style={{fontWeight: 'bold'}} >{'Users'}
+
{'Last Run'}
Date: Wed, 8 Jan 2025 15:30:43 +0100 Subject: [PATCH 06/49] style: Adjust margin of "Run legal hold" button to match other buttons --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index f0f7dd5..c0ef3e0 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -131,7 +131,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { href={downloadUrl} download={true} style={{ - marginRight: '20px', + marginRight: '10px', height: '24px', }} > From 6bd240819140ad2a6160bbe8b183daa3bc0293da Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 8 Jan 2025 16:13:19 +0100 Subject: [PATCH 07/49] feat: Add LastMessageAt attribute to track latest message in LegalHold --- server/legalhold/legal_hold.go | 7 +++++++ server/model/legal_hold.go | 1 + .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 3 +++ .../src/components/legal_hold_table/legal_hold_table.tsx | 6 +++++- 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 048ec19..d791ab4 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -155,6 +155,13 @@ func (ex *Execution) ExportData() error { break } + // Update LastMessageAt if we have newer messages + for _, post := range posts { + if post.PostCreateAt > ex.LegalHold.LastMessageAt { + ex.LegalHold.LastMessageAt = post.PostCreateAt + } + } + err = ex.WritePostsBatchToFile(channelID, posts) if err != nil { return err diff --git a/server/model/legal_hold.go b/server/model/legal_hold.go index 2923599..e803e13 100644 --- a/server/model/legal_hold.go +++ b/server/model/legal_hold.go @@ -21,6 +21,7 @@ type LegalHold struct { EndsAt int64 `json:"ends_at"` IncludePublicChannels bool `json:"include_public_channels"` LastExecutionEndedAt int64 `json:"last_execution_ended_at"` + LastMessageAt int64 `json:"last_message_at"` ExecutionLength int64 `json:"execution_length"` Secret string `json:"secret"` } diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index c0ef3e0..ae981ea 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -48,6 +48,9 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'}
+
+ {lh.last_message_at ? new Date(lh.last_message_at).toLocaleString() : 'No messages'} +
{ aria-label='Legal Holds Table' style={{ display: 'grid', - gridTemplateColumns: 'auto auto auto auto auto auto', + gridTemplateColumns: 'auto auto auto auto auto auto auto', columnGap: '10px', rowGap: '10px', alignItems: 'center', @@ -61,6 +61,10 @@ const LegalHoldTable = (props: LegalHoldTableProps) => { aria-label='actions header' style={{fontWeight: 'bold'}} >{'Last Run'}
+
{'Last Message'}
Date: Wed, 8 Jan 2025 16:34:55 +0100 Subject: [PATCH 08/49] fix: Add missing fields to LegalHold TypeScript interface --- webapp/src/types/index.d.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/webapp/src/types/index.d.ts b/webapp/src/types/index.d.ts index 69f0960..a6d2895 100644 --- a/webapp/src/types/index.d.ts +++ b/webapp/src/types/index.d.ts @@ -7,6 +7,8 @@ export interface LegalHold { user_ids: string[]; include_public_channels: boolean; secret: string; + last_execution_ended_at: number; + last_message_at: number; } export interface CreateLegalHold { From 28a84f8e10c449a95f79adb017e0bc85a90f2a56 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Thu, 9 Jan 2025 08:11:24 +0100 Subject: [PATCH 09/49] fix: use LockWithContext to get locking errors rather than queuing --- server/legalhold/legal_hold.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index d791ab4..73c2653 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -2,6 +2,7 @@ package legalhold import ( "bytes" + "context" "crypto/hmac" "crypto/sha512" "encoding/json" @@ -20,6 +21,7 @@ import ( ) const PostExportBatchLimit = 10000 +const executionGlobalTimeout = 120 * 60 * 1000 // 2 hours // Execution represents one execution of a LegalHold, i.e. a daily (or other duration) // batch process to hold all data relating to that particular LegalHold. It is defined by the @@ -63,7 +65,12 @@ func (ex *Execution) Execute() (int64, error) { return 0, fmt.Errorf("failed to create cluster mutex: %w", err) } - mutex.Lock() + ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout) + defer cancel() + + if err := mutex.LockWithContext(ctx); err != nil { + return 0, fmt.Errorf("failed to lock cluster mutex: %w", err) + } defer mutex.Unlock() err = ex.GetChannels() From a20034d3be01e4f48683493cdcb0ddb0cb41bb4b Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Thu, 9 Jan 2025 13:26:53 +0100 Subject: [PATCH 10/49] feat: Add Status field to LegalHold with idle and executing states --- server/model/legal_hold.go | 36 +++++++++++++++++++++++------------- webapp/src/types/index.d.ts | 1 + 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/server/model/legal_hold.go b/server/model/legal_hold.go index e803e13..e1e6b0a 100644 --- a/server/model/legal_hold.go +++ b/server/model/legal_hold.go @@ -10,20 +10,28 @@ import ( ) // LegalHold represents one legal hold. +type LegalHoldStatus string + +const ( + LegalHoldStatusIdle LegalHoldStatus = "idle" + LegalHoldStatusExecuting LegalHoldStatus = "executing" +) + type LegalHold struct { - ID string `json:"id"` - Name string `json:"name"` - DisplayName string `json:"display_name"` - CreateAt int64 `json:"create_at"` - UpdateAt int64 `json:"update_at"` - UserIDs []string `json:"user_ids"` - StartsAt int64 `json:"starts_at"` - EndsAt int64 `json:"ends_at"` - IncludePublicChannels bool `json:"include_public_channels"` - LastExecutionEndedAt int64 `json:"last_execution_ended_at"` - LastMessageAt int64 `json:"last_message_at"` - ExecutionLength int64 `json:"execution_length"` - Secret string `json:"secret"` + ID string `json:"id"` + Name string `json:"name"` + DisplayName string `json:"display_name"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + UserIDs []string `json:"user_ids"` + StartsAt int64 `json:"starts_at"` + EndsAt int64 `json:"ends_at"` + IncludePublicChannels bool `json:"include_public_channels"` + LastExecutionEndedAt int64 `json:"last_execution_ended_at"` + LastMessageAt int64 `json:"last_message_at"` + ExecutionLength int64 `json:"execution_length"` + Secret string `json:"secret"` + Status LegalHoldStatus `json:"status"` } // DeepCopy creates a deep copy of the LegalHold. @@ -44,6 +52,7 @@ func (lh *LegalHold) DeepCopy() LegalHold { LastExecutionEndedAt: lh.LastExecutionEndedAt, ExecutionLength: lh.ExecutionLength, Secret: lh.Secret, + Status: lh.Status, } if len(lh.UserIDs) > 0 { @@ -159,6 +168,7 @@ func NewLegalHoldFromCreate(lhc CreateLegalHold) LegalHold { IncludePublicChannels: lhc.IncludePublicChannels, LastExecutionEndedAt: 0, ExecutionLength: 86400000, + Status: LegalHoldStatusIdle, } } diff --git a/webapp/src/types/index.d.ts b/webapp/src/types/index.d.ts index a6d2895..8c290d4 100644 --- a/webapp/src/types/index.d.ts +++ b/webapp/src/types/index.d.ts @@ -9,6 +9,7 @@ export interface LegalHold { secret: string; last_execution_ended_at: number; last_message_at: number; + status: 'idle' | 'executing'; } export interface CreateLegalHold { From 14b12db1a55c4099cb59d177b8e94bab88d988ef Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Thu, 9 Jan 2025 15:56:31 +0100 Subject: [PATCH 11/49] feat: Add refresh button to legal holds table header --- .../legal_hold_table/refresh-outline.svg | 3 ++ webapp/src/components/legal_holds_setting.tsx | 34 +++++++++++++++---- .../legal_holds_setting/refresh-outline.svg | 3 ++ 3 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 webapp/src/components/legal_hold_table/refresh-outline.svg create mode 100644 webapp/src/components/legal_holds_setting/refresh-outline.svg diff --git a/webapp/src/components/legal_hold_table/refresh-outline.svg b/webapp/src/components/legal_hold_table/refresh-outline.svg new file mode 100644 index 0000000..2be97ae --- /dev/null +++ b/webapp/src/components/legal_hold_table/refresh-outline.svg @@ -0,0 +1,3 @@ + + + diff --git a/webapp/src/components/legal_holds_setting.tsx b/webapp/src/components/legal_holds_setting.tsx index b0ef064..f4dfb61 100644 --- a/webapp/src/components/legal_holds_setting.tsx +++ b/webapp/src/components/legal_holds_setting.tsx @@ -13,6 +13,7 @@ import ShowSecretModal from '@/components/show_secret_modal'; import ConfirmRelease from '@/components/confirm_release'; import LegalHoldIcon from '@/components/legal_hold_icon.svg'; +import RefreshIcon from '@/components/legal_holds_setting/refresh-outline.svg'; const LegalHoldsSetting = () => { const [legalHoldsFetched, setLegalHoldsFetched] = useState(false); @@ -109,8 +110,8 @@ const LegalHoldsSetting = () => {
{ > {'Legal Holds'}
- setShowCreateModal(true)} - dataTestId='createNewLegalHoldOnTop' - /> +
+ + setShowCreateModal(true)} + dataTestId='createNewLegalHoldOnTop' + /> +
diff --git a/webapp/src/components/legal_holds_setting/refresh-outline.svg b/webapp/src/components/legal_holds_setting/refresh-outline.svg new file mode 100644 index 0000000..2be97ae --- /dev/null +++ b/webapp/src/components/legal_holds_setting/refresh-outline.svg @@ -0,0 +1,3 @@ + + + From d9fbf7d78e3827614a43f6e4ed3f34b9fc54f8f4 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Thu, 9 Jan 2025 16:54:29 +0100 Subject: [PATCH 12/49] feat: Update legal hold execution to track and update status --- server/jobs/legal_hold_job.go | 2 +- server/legalhold/legal_hold.go | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 4cfd80d..0b77735 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -233,7 +233,7 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { } 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) + lhe := legalhold.NewExecution(lh, j.papi, j.sqlstore, j.kvstore, j.filebackend) if end, err := lhe.Execute(); err != nil { if strings.Contains(err.Error(), "another execution is already running") { diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 73c2653..0150e39 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -34,6 +34,7 @@ type Execution struct { papi plugin.API store *sqlstore.SQLStore + kvstore kvstore.KVStore fileBackend filestore.FileBackend channelIDs []string @@ -43,12 +44,13 @@ type Execution struct { } // NewExecution creates a new Execution that is ready to use. -func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQLStore, fileBackend filestore.FileBackend) Execution { +func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQLStore, kvstore kvstore.KVStore, fileBackend filestore.FileBackend) Execution { return Execution{ LegalHold: legalHold, ExecutionStartTime: legalHold.NextExecutionStartTime(), ExecutionEndTime: legalHold.NextExecutionEndTime(), store: store, + kvstore: kvstore, fileBackend: fileBackend, index: model.NewLegalHoldIndex(), papi: papi, @@ -58,6 +60,13 @@ func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQ // Execute executes the Execution. func (ex *Execution) Execute() (int64, error) { + // Set status to executing + ex.LegalHold.Status = model.LegalHoldStatusExecuting + _, err := ex.store.kvstore.UpdateLegalHold(ex.LegalHold, ex.LegalHold) + if err != nil { + return 0, fmt.Errorf("failed to update legal hold status: %w", err) + } + // Lock multiple executions using a cluster mutex mutexKey := fmt.Sprintf("legal_hold_%s_execution", ex.LegalHold.ID) mutex, err := cluster.NewMutex(ex.papi, mutexKey) @@ -93,6 +102,13 @@ func (ex *Execution) Execute() (int64, error) { return 0, err } + // Set status back to idle + ex.LegalHold.Status = model.LegalHoldStatusIdle + _, err = ex.store.kvstore.UpdateLegalHold(ex.LegalHold, ex.LegalHold) + if err != nil { + return 0, fmt.Errorf("failed to update legal hold status: %w", err) + } + return ex.ExecutionEndTime, nil } From 544087dc0eaec37f1172e4a9962f5cd2e231960b Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Thu, 9 Jan 2025 16:59:47 +0100 Subject: [PATCH 13/49] refactor: Add helper method to update legal hold status atomically --- server/legalhold/legal_hold.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 0150e39..2dcc4ed 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -61,8 +61,7 @@ func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQ // Execute executes the Execution. func (ex *Execution) Execute() (int64, error) { // Set status to executing - ex.LegalHold.Status = model.LegalHoldStatusExecuting - _, err := ex.store.kvstore.UpdateLegalHold(ex.LegalHold, ex.LegalHold) + err := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusExecuting) if err != nil { return 0, fmt.Errorf("failed to update legal hold status: %w", err) } @@ -103,8 +102,7 @@ func (ex *Execution) Execute() (int64, error) { } // Set status back to idle - ex.LegalHold.Status = model.LegalHoldStatusIdle - _, err = ex.store.kvstore.UpdateLegalHold(ex.LegalHold, ex.LegalHold) + err = ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) if err != nil { return 0, fmt.Errorf("failed to update legal hold status: %w", err) } From d0f56fe0e05300f7a8af0be7ea85bd79a1a9822f Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Thu, 9 Jan 2025 17:00:05 +0100 Subject: [PATCH 14/49] feat: Add UpdateLegalHoldStatus method to legal hold implementation --- server/legalhold/legal_hold.go | 29 ++++++++++++++++------------- server/store/kvstore/kvstore.go | 1 + server/store/kvstore/legal_hold.go | 23 +++++++++++++++++++++++ 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 2dcc4ed..2862eac 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -16,6 +16,7 @@ import ( "github.com/mattermost/mattermost-server/v6/shared/filestore" "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" "github.com/mattermost/mattermost-plugin-legal-hold/server/utils" ) @@ -60,12 +61,6 @@ func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQ // Execute executes the Execution. func (ex *Execution) Execute() (int64, error) { - // Set status to executing - err := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusExecuting) - if err != nil { - return 0, fmt.Errorf("failed to update legal hold status: %w", err) - } - // Lock multiple executions using a cluster mutex mutexKey := fmt.Sprintf("legal_hold_%s_execution", ex.LegalHold.ID) mutex, err := cluster.NewMutex(ex.papi, mutexKey) @@ -79,7 +74,21 @@ func (ex *Execution) Execute() (int64, error) { if err := mutex.LockWithContext(ctx); err != nil { return 0, fmt.Errorf("failed to lock cluster mutex: %w", err) } - defer mutex.Unlock() + defer func() { + mutex.Unlock() + + // Set status back to idle + _, err := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) + if err != nil { + ex.papi.LogError(fmt.Sprintf("failed to update legal hold status: %v", err)) + } + }() + + // Set status to executing + _, err = ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusExecuting) + if err != nil { + return 0, fmt.Errorf("failed to update legal hold status: %w", err) + } err = ex.GetChannels() if err != nil { @@ -101,12 +110,6 @@ func (ex *Execution) Execute() (int64, error) { return 0, err } - // Set status back to idle - err = ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) - if err != nil { - return 0, fmt.Errorf("failed to update legal hold status: %w", err) - } - return ex.ExecutionEndTime, nil } diff --git a/server/store/kvstore/kvstore.go b/server/store/kvstore/kvstore.go index ad01f07..ecdfbfa 100644 --- a/server/store/kvstore/kvstore.go +++ b/server/store/kvstore/kvstore.go @@ -8,4 +8,5 @@ type KVStore interface { GetLegalHoldByID(id string) (*model.LegalHold, error) UpdateLegalHold(lh, oldValue model.LegalHold) (*model.LegalHold, error) DeleteLegalHold(id string) error + UpdateLegalHoldStatus(id string, status model.LegalHoldStatus) (*model.LegalHold, error) } diff --git a/server/store/kvstore/legal_hold.go b/server/store/kvstore/legal_hold.go index 83a1b86..1d866b5 100644 --- a/server/store/kvstore/legal_hold.go +++ b/server/store/kvstore/legal_hold.go @@ -81,6 +81,29 @@ func (kvs Impl) GetLegalHoldByID(id string) (*model.LegalHold, error) { return &legalHold, nil } +func (kvs Impl) UpdateLegalHoldStatus(id string, status model.LegalHoldStatus) error { + lh, err := kvs.GetLegalHoldByID(id) + if err != nil { + return fmt.Errorf("failed to get legal hold: %w", err) + } + if lh == nil { + return fmt.Errorf("legal hold not found: %s", id) + } + + oldValue := *lh + lh.Status = status + lh.UpdateAt = mattermostModel.GetMillis() + + key := fmt.Sprintf("%s%s", legalHoldPrefix, lh.ID) + saved, err := kvs.client.KV.Set(key, lh, pluginapi.SetAtomic(oldValue)) + if !saved && err != nil { + return errors.Wrap(err, "database error occurred updating legal hold status") + } else if !saved && err == nil { + return errors.New("could not update legal hold status as it has already been updated by someone else") + } + return nil +} + func (kvs Impl) UpdateLegalHold(lh, oldValue model.LegalHold) (*model.LegalHold, error) { lh.UpdateAt = mattermostModel.GetMillis() From ee1d0dd4e66019136b52c6e4cc11806e8c9c81ce Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Thu, 9 Jan 2025 17:20:58 +0100 Subject: [PATCH 15/49] test: Add comprehensive tests for UpdateLegalHoldStatus method --- server/store/kvstore/legal_hold_test.go | 59 +++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/server/store/kvstore/legal_hold_test.go b/server/store/kvstore/legal_hold_test.go index 138f3d2..70c5186 100644 --- a/server/store/kvstore/legal_hold_test.go +++ b/server/store/kvstore/legal_hold_test.go @@ -154,6 +154,65 @@ func TestKVStore_GetAllLegalHolds(t *testing.T) { require.Len(t, result, 0) } +func TestKVStore_UpdateLegalHoldStatus(t *testing.T) { + api := &plugintest.API{} + driver := &plugintest.Driver{} + client := pluginapi.NewClient(api, driver) + + kvstore := NewKVStore(client) + + // Test successful status update + lh1 := model.LegalHold{ + ID: mattermostModel.NewId(), + Name: "legal-hold-1", + Status: model.LegalHoldStatusIdle, + } + marshaled, err := json.Marshal(lh1) + require.NoError(t, err) + + api.On("KVGet", fmt.Sprintf("%s%s", legalHoldPrefix, lh1.ID)). + Return(marshaled, nil).Once() + + api.On("KVSetWithOptions", + mock.AnythingOfType("string"), + mock.AnythingOfType("[]uint8"), + mock.AnythingOfType("model.PluginKVSetOptions"), + ).Return(true, nil).Once() + + err = kvstore.UpdateLegalHoldStatus(lh1.ID, model.LegalHoldStatusExecuting) + require.NoError(t, err) + + // Test with non-existent legal hold + api.On("KVGet", mock.AnythingOfType("string")). + Return(nil, &mattermostModel.AppError{}).Once() + + err = kvstore.UpdateLegalHoldStatus("doesnotexist", model.LegalHoldStatusExecuting) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to get legal hold") + + // Test with atomic update failure + lh2 := model.LegalHold{ + ID: mattermostModel.NewId(), + Name: "legal-hold-2", + Status: model.LegalHoldStatusIdle, + } + marshaled2, err := json.Marshal(lh2) + require.NoError(t, err) + + api.On("KVGet", fmt.Sprintf("%s%s", legalHoldPrefix, lh2.ID)). + Return(marshaled2, nil).Once() + + api.On("KVSetWithOptions", + mock.AnythingOfType("string"), + mock.AnythingOfType("[]uint8"), + mock.AnythingOfType("model.PluginKVSetOptions"), + ).Return(false, nil).Once() + + err = kvstore.UpdateLegalHoldStatus(lh2.ID, model.LegalHoldStatusExecuting) + require.Error(t, err) + require.Contains(t, err.Error(), "already been updated by someone else") +} + func TestKVStore_UpdateLegalHold(t *testing.T) { api := &plugintest.API{} driver := &plugintest.Driver{} From 21369ebf27fcc92f3cc767d296a214937f2bf6c1 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 09:30:27 +0100 Subject: [PATCH 16/49] refactor: Update legal hold status method signature to return only error --- server/legalhold/legal_hold.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 2862eac..d8b59b7 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -78,14 +78,14 @@ func (ex *Execution) Execute() (int64, error) { mutex.Unlock() // Set status back to idle - _, err := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) + err := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) if err != nil { ex.papi.LogError(fmt.Sprintf("failed to update legal hold status: %v", err)) } }() // Set status to executing - _, err = ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusExecuting) + err = ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusExecuting) if err != nil { return 0, fmt.Errorf("failed to update legal hold status: %w", err) } From 1cb2ed82eb60a402dc1df80c35914fe4a6340349 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 09:30:31 +0100 Subject: [PATCH 17/49] refactor: Update Execute method to return updated LegalHold instead of end time --- server/jobs/legal_hold_job.go | 11 ++--------- server/legalhold/legal_hold.go | 22 ++++++++++++---------- server/store/kvstore/kvstore.go | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 0b77735..617eab5 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -235,21 +235,14 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { 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 end, err := lhe.Execute(); err != nil { + 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.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) + newLH, err := j.kvstore.UpdateLegalHold(*updatedLH, lh) if err != nil { j.client.Log.Error("Failed to update legal hold", err) continue diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index d8b59b7..da1f95e 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -59,20 +59,20 @@ func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQ } } -// Execute executes the Execution. -func (ex *Execution) Execute() (int64, error) { +// Execute executes the Execution and returns the updated LegalHold. +func (ex *Execution) Execute() (*model.LegalHold, error) { // Lock multiple executions using a cluster mutex mutexKey := fmt.Sprintf("legal_hold_%s_execution", ex.LegalHold.ID) mutex, err := cluster.NewMutex(ex.papi, mutexKey) if err != nil { - return 0, fmt.Errorf("failed to create cluster mutex: %w", err) + return nil, fmt.Errorf("failed to create cluster mutex: %w", err) } ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout) defer cancel() if err := mutex.LockWithContext(ctx); err != nil { - return 0, fmt.Errorf("failed to lock cluster mutex: %w", err) + return nil, fmt.Errorf("failed to lock cluster mutex: %w", err) } defer func() { mutex.Unlock() @@ -87,30 +87,32 @@ func (ex *Execution) Execute() (int64, error) { // Set status to executing err = ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusExecuting) if err != nil { - return 0, fmt.Errorf("failed to update legal hold status: %w", err) + return nil, fmt.Errorf("failed to update legal hold status: %w", err) } err = ex.GetChannels() if err != nil { - return 0, err + return nil, err } err = ex.ExportData() if err != nil { - return 0, err + return nil, err } err = ex.UpdateIndexes() if err != nil { - return 0, err + return nil, err } err = ex.WriteFileHashes() if err != nil { - return 0, err + return nil, err } - return ex.ExecutionEndTime, nil + // Update the LegalHold with execution results + ex.LegalHold.LastExecutionEndedAt = ex.ExecutionEndTime + return &ex.LegalHold, nil } // GetChannels populates the list of channels that the Execution needs to cover within the diff --git a/server/store/kvstore/kvstore.go b/server/store/kvstore/kvstore.go index ecdfbfa..c0433d6 100644 --- a/server/store/kvstore/kvstore.go +++ b/server/store/kvstore/kvstore.go @@ -8,5 +8,5 @@ type KVStore interface { GetLegalHoldByID(id string) (*model.LegalHold, error) UpdateLegalHold(lh, oldValue model.LegalHold) (*model.LegalHold, error) DeleteLegalHold(id string) error - UpdateLegalHoldStatus(id string, status model.LegalHoldStatus) (*model.LegalHold, error) + UpdateLegalHoldStatus(id string, status model.LegalHoldStatus) error } From 010c08267306a14ccf77153605dca21f1e5db00a Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 09:38:58 +0100 Subject: [PATCH 18/49] feat: Disable download and run buttons based on legal hold status --- .../legal_hold_row/legal_hold_row.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index ae981ea..f03c8e8 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -131,11 +131,18 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { { + if (lh.last_message_at === 0) { + e.preventDefault(); + } + }} style={{ marginRight: '10px', height: '24px', + opacity: lh.last_message_at === 0 ? '0.5' : '1', + cursor: lh.last_message_at === 0 ? 'not-allowed' : 'pointer', }} > { data-testid={`run-${lh.id}`} aria-label={`${lh.display_name} run button`} href='#' - onClick={() => setShowRunConfirmModal(true)} + onClick={(e) => { + e.preventDefault(); + if (lh.status !== 'executing') { + setShowRunConfirmModal(true); + } + }} style={{ marginRight: '20px', height: '24px', + opacity: lh.status === 'executing' ? '0.5' : '1', + cursor: lh.status === 'executing' ? 'not-allowed' : 'pointer', }} > Date: Mon, 13 Jan 2025 09:48:54 +0100 Subject: [PATCH 19/49] refactor: Simplify run confirmation modal trigger logic --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index f03c8e8..7608a8b 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -171,9 +171,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { href='#' onClick={(e) => { e.preventDefault(); - if (lh.status !== 'executing') { - setShowRunConfirmModal(true); - } + setShowRunConfirmModal(true); }} style={{ marginRight: '20px', From 525aabe2b11f5376b4a02ec255d1c6571404d422 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 09:48:56 +0100 Subject: [PATCH 20/49] feat: Show "Running..." for legal hold when status is executing --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 7608a8b..9db849c 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -46,7 +46,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{endsAt}
{props.users.length} {'users'}
- {lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'} + {lh.status === 'executing' ? 'Running...' : (lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never')}
{lh.last_message_at ? new Date(lh.last_message_at).toLocaleString() : 'No messages'} From adffbf1e5b1c1ef9c1040ec110b9df424f9e9695 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 09:49:40 +0100 Subject: [PATCH 21/49] refactor: Extract display logic for last run and message into separate functions --- .../legal_hold_row/legal_hold_row.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 9db849c..0c4d29f 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -23,6 +23,17 @@ interface LegalHoldRowProps { runLegalHold: (id: string) => Promise; } +const getLastRunDisplay = (lh: LegalHold) => { + if (lh.status === 'executing') { + return 'Running...'; + } + return lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'; +}; + +const getLastMessageDisplay = (lh: LegalHold) => { + return lh.last_message_at ? new Date(lh.last_message_at).toLocaleString() : 'No messages'; +}; + const LegalHoldRow = (props: LegalHoldRowProps) => { const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); const [showRunErrorModal, setShowRunErrorModal] = useState(false); @@ -46,10 +57,10 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{endsAt}
{props.users.length} {'users'}
- {lh.status === 'executing' ? 'Running...' : (lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never')} + {getLastRunDisplay(lh)}
- {lh.last_message_at ? new Date(lh.last_message_at).toLocaleString() : 'No messages'} + {getLastMessageDisplay(lh)}
Date: Mon, 13 Jan 2025 10:34:25 +0100 Subject: [PATCH 22/49] fix: ensure updates to legal hold attributes correctly --- server/jobs/legal_hold_job.go | 17 ++++++++++++++--- server/legalhold/legal_hold.go | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 617eab5..76c97cf 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -178,6 +178,8 @@ 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 { @@ -213,8 +215,6 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { settings = j.settings.Clone() j.mux.Unlock() - j.client.Log.Info("Processing all Legal Holds") - for _, lh := range legalHolds { for { if lh.IsFinished() { @@ -242,7 +242,18 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { } j.client.Log.Error("An error occurred executing the legal hold.", err) } else { - newLH, err := j.kvstore.UpdateLegalHold(*updatedLH, lh) + // 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 diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index da1f95e..430e325 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -112,6 +112,7 @@ func (ex *Execution) Execute() (*model.LegalHold, error) { // Update the LegalHold with execution results ex.LegalHold.LastExecutionEndedAt = ex.ExecutionEndTime + ex.LegalHold.Status = model.LegalHoldStatusIdle return &ex.LegalHold, nil } From 616219aee0f6a13f7a8a7648bf2e2fb79d80698c Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 10:34:44 +0100 Subject: [PATCH 23/49] fix: execution global timeout type --- server/legalhold/legal_hold.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 430e325..d06246b 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "strings" + "time" "github.com/gocarina/gocsv" "github.com/mattermost/mattermost-plugin-api/cluster" @@ -68,7 +69,7 @@ func (ex *Execution) Execute() (*model.LegalHold, error) { return nil, fmt.Errorf("failed to create cluster mutex: %w", err) } - ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout) + ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout*time.Second) defer cancel() if err := mutex.LockWithContext(ctx); err != nil { From 54b2c6c3221c1c6b3ea9e393c54d2b70ca45e934 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 10:38:29 +0100 Subject: [PATCH 24/49] feat: Add refresh functionality after running legal hold --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 3 ++- webapp/src/components/legal_hold_table/legal_hold_table.tsx | 2 ++ webapp/src/components/legal_holds_setting.tsx | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 0c4d29f..c7a5f0d 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -21,6 +21,7 @@ interface LegalHoldRowProps { showUpdateModal: Function; showSecretModal: Function; runLegalHold: (id: string) => Promise; + refresh: () => void; } const getLastRunDisplay = (lh: LegalHold) => { @@ -215,7 +216,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { onConfirm={() => { setShowRunConfirmModal(false); props.runLegalHold(lh.id).then(() => { - // Success - do nothing as the message from the API is enough + props.refresh(); }).catch(() => { setShowRunErrorModal(true); }); diff --git a/webapp/src/components/legal_hold_table/legal_hold_table.tsx b/webapp/src/components/legal_hold_table/legal_hold_table.tsx index fe9cc4e..1ebf3c5 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_table.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_table.tsx @@ -12,6 +12,7 @@ interface LegalHoldTableProps { showUpdateModal: Function, showSecretModal: Function, runLegalHold: (id: string) => Promise; + refresh: () => void; } const LegalHoldTable = (props: LegalHoldTableProps) => { @@ -78,6 +79,7 @@ const LegalHoldTable = (props: LegalHoldTableProps) => { showUpdateModal={props.showUpdateModal} showSecretModal={props.showSecretModal} runLegalHold={(id: string) => props.runLegalHold(id)} + refresh={props.refresh} /> ); })} diff --git a/webapp/src/components/legal_holds_setting.tsx b/webapp/src/components/legal_holds_setting.tsx index f4dfb61..1d4ebab 100644 --- a/webapp/src/components/legal_holds_setting.tsx +++ b/webapp/src/components/legal_holds_setting.tsx @@ -206,6 +206,7 @@ const LegalHoldsSetting = () => { showUpdateModal={doShowUpdateModal} showSecretModal={doShowSecretModal} runLegalHold={doRunLegalHold} + refresh={() => setLegalHoldsFetched(false)} /> )} From ba0a5cf30ca3edf8f7ab30e6a31b2cf5387aeac3 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 10:40:40 +0100 Subject: [PATCH 25/49] fix: Resolve golangci-lint shadowing warnings in legal_hold.go --- server/legalhold/legal_hold.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index d06246b..c2f3f49 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -72,16 +72,16 @@ func (ex *Execution) Execute() (*model.LegalHold, error) { ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout*time.Second) defer cancel() - if err := mutex.LockWithContext(ctx); err != nil { - return nil, fmt.Errorf("failed to lock cluster mutex: %w", err) + if lockErr := mutex.LockWithContext(ctx); lockErr != nil { + return nil, fmt.Errorf("failed to lock cluster mutex: %w", lockErr) } defer func() { mutex.Unlock() // Set status back to idle - err := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) - if err != nil { - ex.papi.LogError(fmt.Sprintf("failed to update legal hold status: %v", err)) + statusErr := ex.kvstore.UpdateLegalHoldStatus(ex.LegalHold.ID, model.LegalHoldStatusIdle) + if statusErr != nil { + ex.papi.LogError(fmt.Sprintf("failed to update legal hold status: %v", statusErr)) } }() From cf06433a38ee58848b3bb920c935c0620073461f Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 10:41:17 +0100 Subject: [PATCH 26/49] fix: disallow run button if legal hold is executing --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index c7a5f0d..5ac2662 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -183,6 +183,9 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { href='#' onClick={(e) => { e.preventDefault(); + if (lh.status === 'executing') { + return; + } setShowRunConfirmModal(true); }} style={{ From 7e2363bf8550890d8b9fb255131470599322b681 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 11:13:55 +0100 Subject: [PATCH 27/49] fix: force condition --- server/jobs/legal_hold_job.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 76c97cf..7e0e8e2 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -227,7 +227,7 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { j.client.Log.Debug(fmt.Sprintf("Legal Hold %s is not yet ready to be executed again.", lh.ID)) break } - if forceRun && lh.LastExecutionEndedAt >= now { + if !forceRun && lh.LastExecutionEndedAt >= now { j.client.Log.Debug(fmt.Sprintf("Legal Hold %s was already executed after the current time.", lh.ID)) break } From 49746ff242d75120cd89e9b5e2523ddc78d10e23 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 11:25:18 +0100 Subject: [PATCH 28/49] fix: avoid double loop, continue with next legal holds on error --- server/jobs/legal_hold_job.go | 80 +++++++++++++++++------------------ 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/server/jobs/legal_hold_job.go b/server/jobs/legal_hold_job.go index 7e0e8e2..20147b7 100644 --- a/server/jobs/legal_hold_job.go +++ b/server/jobs/legal_hold_job.go @@ -216,51 +216,49 @@ func (j *LegalHoldJob) runWith(legalHolds []model.LegalHold, forceRun bool) { j.mux.Unlock() for _, lh := range legalHolds { - for { - if lh.IsFinished() { - j.client.Log.Debug(fmt.Sprintf("Legal Hold %s has ended and therefore does not executing.", lh.ID)) - break - } + if lh.IsFinished() { + j.client.Log.Debug(fmt.Sprintf("Legal Hold %s has ended and therefore does not executing.", lh.ID)) + continue + } - 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)) - break + 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 + } + + 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 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 } - if !forceRun && lh.LastExecutionEndedAt >= now { - j.client.Log.Debug(fmt.Sprintf("Legal Hold %s was already executed after the current time.", lh.ID)) - break + 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 } - - 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 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.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) + 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 From 646435b176479f5fc96dd4e7f84467c65015a6f1 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 11:26:32 +0100 Subject: [PATCH 29/49] feat: Add endpoint to reset legal hold status to IDLE feat: Add reset status endpoint with five-click trigger for legal hold fix: Import Client in legal_hold_row.tsx to resolve ReferenceError --- server/api.go | 33 +++++++++++++++++++ webapp/src/client.ts | 5 +++ .../legal_hold_row/legal_hold_row.tsx | 21 +++++++++++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/server/api.go b/server/api.go index e724352..4b187a7 100644 --- a/server/api.go +++ b/server/api.go @@ -45,6 +45,7 @@ func (p *Plugin) ServeHTTP(_ *plugin.Context, w http.ResponseWriter, r *http.Req 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 @@ -354,6 +355,38 @@ func (p *Plugin) runSingleLegalHold(w http.ResponseWriter, r *http.Request) { } // testAmazonS3Connection tests the plugin's custom Amazon S3 connection +func (p *Plugin) resetLegalHoldStatus(w http.ResponseWriter, r *http.Request) { + 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()) + } +} + func (p *Plugin) testAmazonS3Connection(w http.ResponseWriter, _ *http.Request) { type messageResponse struct { Message string `json:"message"` diff --git a/webapp/src/client.ts b/webapp/src/client.ts index 1c8d141..cb97172 100644 --- a/webapp/src/client.ts +++ b/webapp/src/client.ts @@ -37,6 +37,11 @@ class APIClient { return this.doWithBody(url, 'post', {}); }; + resetLegalHoldStatus = (id: string) => { + const url = `${this.url}/legalholds/${id}/resetstatus`; + return this.doWithBody(url, 'post', {}); + }; + testAmazonS3Connection = () => { const url = `${this.url}/test_amazon_s3_connection`; return this.doWithBody(url, 'post', {}) as Promise<{message: string}>; diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 5ac2662..c96622a 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -1,6 +1,7 @@ import React, {useState} from 'react'; import {UserProfile} from 'mattermost-redux/types/users'; +import Client from '@/client'; import {LegalHold} from '@/types'; import Tooltip from '@/components/mattermost-webapp/tooltip'; @@ -38,6 +39,7 @@ const getLastMessageDisplay = (lh: LegalHold) => { const LegalHoldRow = (props: LegalHoldRowProps) => { const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); const [showRunErrorModal, setShowRunErrorModal] = useState(false); + const [resetClickCount, setResetClickCount] = useState(0); const lh = props.legalHold; const startsAt = (new Date(lh.starts_at)).toLocaleDateString(); const endsAt = lh.ends_at === 0 ? 'Never' : (new Date(lh.ends_at)).toLocaleDateString(); @@ -57,7 +59,24 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{startsAt}
{endsAt}
{props.users.length} {'users'}
-
+
{ + const newCount = resetClickCount + 1; + setResetClickCount(newCount); + if (newCount === 5) { + setResetClickCount(0); + Client.resetLegalHoldStatus(lh.id) + .then(() => { + props.refresh(); + }) + .catch(() => { + // Silently fail + }); + } + }} + style={{cursor: 'pointer'}} + > {getLastRunDisplay(lh)}
From 7853d55f74e54ea62b79fb12f20a847d7808b027 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Mon, 13 Jan 2025 11:32:52 +0100 Subject: [PATCH 30/49] refactor: Remove "Last Message" column from legal hold table --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 6 ------ webapp/src/components/legal_hold_table/legal_hold_table.tsx | 6 +----- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index c96622a..f4b0ce0 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -32,9 +32,6 @@ const getLastRunDisplay = (lh: LegalHold) => { return lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'; }; -const getLastMessageDisplay = (lh: LegalHold) => { - return lh.last_message_at ? new Date(lh.last_message_at).toLocaleString() : 'No messages'; -}; const LegalHoldRow = (props: LegalHoldRowProps) => { const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); @@ -79,9 +76,6 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { > {getLastRunDisplay(lh)}
-
- {getLastMessageDisplay(lh)} -
{ aria-label='Legal Holds Table' style={{ display: 'grid', - gridTemplateColumns: 'auto auto auto auto auto auto auto', + gridTemplateColumns: 'auto auto auto auto auto auto', columnGap: '10px', rowGap: '10px', alignItems: 'center', @@ -62,10 +62,6 @@ const LegalHoldTable = (props: LegalHoldTableProps) => { aria-label='actions header' style={{fontWeight: 'bold'}} >{'Last Run'}
-
{'Last Message'}
Date: Mon, 13 Jan 2025 11:41:07 +0100 Subject: [PATCH 31/49] test: Add API tests for resetstatus and run legal hold endpoints test: Remove duplicate import and improve test setup refactor: Add LegalHoldJobInterface to support mock testing fix: Initialize router in tests to resolve 404 errors fix: Add missing gorilla/mux import in api_test.go fix: Resolve 404 errors by correctly initializing plugin router in tests fix: Update test URLs to use hyphens instead of underscores fix: Update test cases to use `testid` instead of `test-id` fix: Use model.NewId() for generating valid test IDs feat: Initialize KVStore in TestResetLegalHoldStatus test fix: Add missing kvstore import in api_test.go --- server/api_test.go | 134 ++++++++++++++++++++++++ server/jobs/legal_hold_job_interface.go | 16 +++ server/plugin.go | 2 +- 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 server/jobs/legal_hold_job_interface.go diff --git a/server/api_test.go b/server/api_test.go index 003ae2a..0b20420 100644 --- a/server/api_test.go +++ b/server/api_test.go @@ -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{} diff --git a/server/jobs/legal_hold_job_interface.go b/server/jobs/legal_hold_job_interface.go new file mode 100644 index 0000000..1c22051 --- /dev/null +++ b/server/jobs/legal_hold_job_interface.go @@ -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 +} diff --git a/server/plugin.go b/server/plugin.go index 2bf389f..dacf7e3 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -49,7 +49,7 @@ type Plugin struct { jobManager *jobs.JobManager // legalHoldJob runs the legal hold jobs - legalHoldJob *jobs.LegalHoldJob + legalHoldJob jobs.LegalHoldJobInterface // router holds the HTTP router for the plugin's rest API router *mux.Router From cfadfd6ff505d90683cc9c39227a664bb231b841 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 15:47:57 +0100 Subject: [PATCH 32/49] fix(aider): some messups in the code --- server/legalhold/legal_hold.go | 23 +++++++++++-------- .../legal_hold_row/legal_hold_row.tsx | 4 ++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/server/legalhold/legal_hold.go b/server/legalhold/legal_hold.go index 5ea71ef..2bfe9d2 100644 --- a/server/legalhold/legal_hold.go +++ b/server/legalhold/legal_hold.go @@ -23,7 +23,7 @@ import ( ) const PostExportBatchLimit = 10000 -const executionGlobalTimeout = 120 * 60 * 1000 // 2 hours +const executionGlobalTimeout = 48 * time.Hour // Execution represents one execution of a LegalHold, i.e. a daily (or other duration) // batch process to hold all data relating to that particular LegalHold. It is defined by the @@ -62,6 +62,8 @@ func NewExecution(legalHold model.LegalHold, papi plugin.API, store *sqlstore.SQ // Execute executes the Execution and returns the updated LegalHold. func (ex *Execution) Execute() (*model.LegalHold, error) { + now := time.Now().Unix() + // Lock multiple executions using a cluster mutex mutexKey := fmt.Sprintf("legal_hold_%s_execution", ex.LegalHold.ID) mutex, err := cluster.NewMutex(ex.papi, mutexKey) @@ -69,7 +71,7 @@ func (ex *Execution) Execute() (*model.LegalHold, error) { return nil, fmt.Errorf("failed to create cluster mutex: %w", err) } - ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), executionGlobalTimeout) defer cancel() if lockErr := mutex.LockWithContext(ctx); lockErr != nil { @@ -113,7 +115,14 @@ func (ex *Execution) Execute() (*model.LegalHold, error) { // Update the LegalHold with execution results ex.LegalHold.LastExecutionEndedAt = ex.ExecutionEndTime + + // Ensure that the LastExecutionEndedAt is not in the future, useful when running the job manually + if ex.ExecutionEndTime > now { + ex.LegalHold.LastExecutionEndedAt = now + } + ex.LegalHold.Status = model.LegalHoldStatusIdle + return &ex.LegalHold, nil } @@ -185,13 +194,6 @@ func (ex *Execution) ExportData() error { break } - // Update LastMessageAt if we have newer messages - for _, post := range posts { - if post.PostCreateAt > ex.LegalHold.LastMessageAt { - ex.LegalHold.LastMessageAt = post.PostCreateAt - } - } - ex.papi.LogDebug("Legal hold executor - ExportData", "channel_id", channelID, "post_count", len(posts)) err = ex.WritePostsBatchToFile(channelID, posts) @@ -199,6 +201,9 @@ func (ex *Execution) ExportData() error { return err } + // Update LastMessageAt with the last CreatedAt post + ex.LegalHold.LastMessageAt = posts[len(posts)-1].PostCreateAt + // Extract the FileIDs to export var fileIDs []string for _, post := range posts { diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index f4b0ce0..9a8c63f 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -45,7 +45,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { props.releaseLegalHold(lh); }; - const downloadUrl = `/plugins/com.mattermost.plugin-legal-hold/api/v1/legalholds/${lh.id}/download`; + const downloadUrl = Client.downloadUrl(lh.id); return ( @@ -56,7 +56,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{startsAt}
{endsAt}
{props.users.length} {'users'}
-
{ const newCount = resetClickCount + 1; From adfafa3141f3c657da95a22017a38e14f43dc443 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 13 Jan 2025 15:54:07 +0100 Subject: [PATCH 33/49] fix: styles --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 9a8c63f..2c97d63 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -32,7 +32,6 @@ const getLastRunDisplay = (lh: LegalHold) => { return lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'; }; - const LegalHoldRow = (props: LegalHoldRowProps) => { const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); const [showRunErrorModal, setShowRunErrorModal] = useState(false); @@ -63,11 +62,11 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { setResetClickCount(newCount); if (newCount === 5) { setResetClickCount(0); - Client.resetLegalHoldStatus(lh.id) - .then(() => { + Client.resetLegalHoldStatus(lh.id). + then(() => { props.refresh(); - }) - .catch(() => { + }). + catch(() => { // Silently fail }); } From 3c89687962fbcc9c30c8cd84cb7fc40730d27d44 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Tue, 14 Jan 2025 10:42:21 +0100 Subject: [PATCH 34/49] chore: fixed comment --- server/api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/api.go b/server/api.go index 4b187a7..a35dd11 100644 --- a/server/api.go +++ b/server/api.go @@ -354,7 +354,7 @@ func (p *Plugin) runSingleLegalHold(w http.ResponseWriter, r *http.Request) { } } -// testAmazonS3Connection tests the plugin's custom Amazon S3 connection +// resetLegalHoldStatus resets the status of a LegalHold to Idle func (p *Plugin) resetLegalHoldStatus(w http.ResponseWriter, r *http.Request) { legalholdID, err := RequireLegalHoldID(r) if err != nil { @@ -387,6 +387,7 @@ func (p *Plugin) resetLegalHoldStatus(w http.ResponseWriter, r *http.Request) { } } +// testAmazonS3Connection tests the plugin's custom Amazon S3 connection func (p *Plugin) testAmazonS3Connection(w http.ResponseWriter, _ *http.Request) { type messageResponse struct { Message string `json:"message"` From 4967cb5f3e20e8fa8813c56a85bda953ffce23ca Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Tue, 14 Jan 2025 10:50:36 +0100 Subject: [PATCH 35/49] feat: Add confirmation modal for legal hold reset action --- .../legal_hold_row/legal_hold_row.tsx | 29 +++++++++---- .../reset_confirmation_modal.tsx | 42 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 2c97d63..996b5cc 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -14,6 +14,7 @@ import EyeLockIcon from './eye-outline_F06D0.svg'; import RunIcon from './play-outline.svg'; import RunConfirmationModal from './run_confirmation_modal'; import RunErrorModal from './run_error_modal'; +import ResetConfirmationModal from './reset_confirmation_modal'; interface LegalHoldRowProps { legalHold: LegalHold; @@ -35,6 +36,7 @@ const getLastRunDisplay = (lh: LegalHold) => { const LegalHoldRow = (props: LegalHoldRowProps) => { const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); const [showRunErrorModal, setShowRunErrorModal] = useState(false); + const [showResetConfirmModal, setShowResetConfirmModal] = useState(false); const [resetClickCount, setResetClickCount] = useState(0); const lh = props.legalHold; const startsAt = (new Date(lh.starts_at)).toLocaleDateString(); @@ -61,14 +63,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { const newCount = resetClickCount + 1; setResetClickCount(newCount); if (newCount === 5) { - setResetClickCount(0); - Client.resetLegalHoldStatus(lh.id). - then(() => { - props.refresh(); - }). - catch(() => { - // Silently fail - }); + setShowResetConfirmModal(true); } }} style={{cursor: 'pointer'}} @@ -241,6 +236,24 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { show={showRunErrorModal} onHide={() => setShowRunErrorModal(false)} /> + { + setShowResetConfirmModal(false); + setResetClickCount(0); + }} + onConfirm={() => { + setShowResetConfirmModal(false); + setResetClickCount(0); + Client.resetLegalHoldStatus(lh.id). + then(() => { + props.refresh(); + }). + catch(() => { + // Silently fail + }); + }} + /> ); }; diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx new file mode 100644 index 0000000..43a5240 --- /dev/null +++ b/webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx @@ -0,0 +1,42 @@ +import React from 'react'; +import {Modal} from 'react-bootstrap'; + +interface Props { + show: boolean; + onHide: () => void; + onConfirm: () => void; +} + +const ResetConfirmationModal = ({show, onHide, onConfirm}: Props) => { + return ( + + + {'Are you sure?'} + + + {'This will modify the legal hold status forcefully setting the status to \'not running\'. Only run this if you know what you are doing.'} + + + + + + + ); +}; + +export default ResetConfirmationModal; From 057ea8245ab4e8c612abe599c5b8a250b3f428b0 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 15 Jan 2025 10:39:02 +0100 Subject: [PATCH 36/49] refactor: Remove click-to-reset functionality from legal hold row --- .../legal_hold_row/legal_hold_row.tsx | 29 ------------- .../reset_confirmation_modal.tsx | 42 ------------------- 2 files changed, 71 deletions(-) delete mode 100644 webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 996b5cc..125b4a6 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -14,7 +14,6 @@ import EyeLockIcon from './eye-outline_F06D0.svg'; import RunIcon from './play-outline.svg'; import RunConfirmationModal from './run_confirmation_modal'; import RunErrorModal from './run_error_modal'; -import ResetConfirmationModal from './reset_confirmation_modal'; interface LegalHoldRowProps { legalHold: LegalHold; @@ -36,8 +35,6 @@ const getLastRunDisplay = (lh: LegalHold) => { const LegalHoldRow = (props: LegalHoldRowProps) => { const [showRunConfirmModal, setShowRunConfirmModal] = useState(false); const [showRunErrorModal, setShowRunErrorModal] = useState(false); - const [showResetConfirmModal, setShowResetConfirmModal] = useState(false); - const [resetClickCount, setResetClickCount] = useState(0); const lh = props.legalHold; const startsAt = (new Date(lh.starts_at)).toLocaleDateString(); const endsAt = lh.ends_at === 0 ? 'Never' : (new Date(lh.ends_at)).toLocaleDateString(); @@ -59,14 +56,6 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
{props.users.length} {'users'}
{ - const newCount = resetClickCount + 1; - setResetClickCount(newCount); - if (newCount === 5) { - setShowResetConfirmModal(true); - } - }} - style={{cursor: 'pointer'}} > {getLastRunDisplay(lh)}
@@ -236,24 +225,6 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { show={showRunErrorModal} onHide={() => setShowRunErrorModal(false)} /> - { - setShowResetConfirmModal(false); - setResetClickCount(0); - }} - onConfirm={() => { - setShowResetConfirmModal(false); - setResetClickCount(0); - Client.resetLegalHoldStatus(lh.id). - then(() => { - props.refresh(); - }). - catch(() => { - // Silently fail - }); - }} - /> ); }; diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx deleted file mode 100644 index 43a5240..0000000 --- a/webapp/src/components/legal_hold_table/legal_hold_row/reset_confirmation_modal.tsx +++ /dev/null @@ -1,42 +0,0 @@ -import React from 'react'; -import {Modal} from 'react-bootstrap'; - -interface Props { - show: boolean; - onHide: () => void; - onConfirm: () => void; -} - -const ResetConfirmationModal = ({show, onHide, onConfirm}: Props) => { - return ( - - - {'Are you sure?'} - - - {'This will modify the legal hold status forcefully setting the status to \'not running\'. Only run this if you know what you are doing.'} - - - - - - - ); -}; - -export default ResetConfirmationModal; From 3388964ede9a6aa137e372cdb18c4e6c47c3f815 Mon Sep 17 00:00:00 2001 From: "Felipe Martin (aider)" Date: Wed, 15 Jan 2025 11:10:18 +0100 Subject: [PATCH 37/49] feat: Reset all legal hold statuses to pending on plugin activation --- server/plugin.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/server/plugin.go b/server/plugin.go index dacf7e3..bddea5d 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -7,13 +7,14 @@ import ( "github.com/gorilla/mux" pluginapi "github.com/mattermost/mattermost-plugin-api" - "github.com/mattermost/mattermost-server/v6/model" + mattermostModel "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/plugin" "github.com/mattermost/mattermost-server/v6/shared/filestore" "github.com/pkg/errors" "github.com/mattermost/mattermost-plugin-legal-hold/server/config" "github.com/mattermost/mattermost-plugin-legal-hold/server/jobs" + "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" ) @@ -84,6 +85,20 @@ func (p *Plugin) OnActivate() error { p.KVStore = kvstore.NewKVStore(p.Client) + // Reset all legal holds on activation + legalHolds, err := p.KVStore.GetAllLegalHolds() + if err != nil { + p.Client.Log.Error("Failed to get legal holds during activation", "err", err) + return err + } + + for _, lh := range legalHolds { + if err := p.KVStore.UpdateLegalHoldStatus(lh.ID, model.LegalHoldStatusIdle); err != nil { + p.Client.Log.Error("Failed to reset legal hold status during activation", "legal_hold_id", lh.ID, "err", err) + return err + } + } + // Create job manager p.jobManager = jobs.NewJobManager(&p.Client.Log) @@ -213,8 +228,8 @@ func (p *Plugin) Reconfigure() error { return nil } -func FixedFileSettingsToFileBackendSettings(fileSettings model.FileSettings) filestore.FileBackendSettings { - if *fileSettings.DriverName == model.ImageDriverLocal { +func FixedFileSettingsToFileBackendSettings(fileSettings mattermostModel.FileSettings) filestore.FileBackendSettings { + if *fileSettings.DriverName == mattermostModel.ImageDriverLocal { return filestore.FileBackendSettings{ DriverName: *fileSettings.DriverName, Directory: *fileSettings.Directory, From 88ec7c71e75a1cfbb2c8156f260ca536c22dd833 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Wed, 15 Jan 2025 12:15:41 +0100 Subject: [PATCH 38/49] chore: added TODO comments for OverlayTrigger --- .../legal_hold_row/legal_hold_row.tsx | 16 ++++++++++++++++ .../components/mattermost-webapp/input/input.tsx | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 125b4a6..0c3d2a7 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -65,6 +65,10 @@ const LegalHoldRow = (props: LegalHoldRowProps) => { alignItems: 'center', }} > + {/* + TODO: Replace when updating the webapp dependency: + https://github.com/mattermost/mattermost-plugin-legal-hold/pull/129#discussion_r1914917354 + */} { + {/* + TODO: Replace when updating the webapp dependency: + https://github.com/mattermost/mattermost-plugin-legal-hold/pull/129#discussion_r1914917354 + */} { + {/* + TODO: Replace when updating the webapp dependency: + https://github.com/mattermost/mattermost-plugin-legal-hold/pull/129#discussion_r1914917354 + */} { + {/* + TODO: Replace when updating the webapp dependency: + https://github.com/mattermost/mattermost-plugin-legal-hold/pull/129#discussion_r1914917354 + */} + {/* + TODO: Replace when updating the webapp dependency: + https://github.com/mattermost/mattermost-plugin-legal-hold/pull/129#discussion_r1914917354 + */} Date: Wed, 15 Jan 2025 12:27:45 +0100 Subject: [PATCH 39/49] feat: Add last execution timestamp to legal hold row display --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index 0c3d2a7..adc80cf 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -57,7 +57,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
- {getLastRunDisplay(lh)} + {getLastRunDisplay(lh)} {lh.last_execution_ended_at}
Date: Wed, 15 Jan 2025 12:27:53 +0100 Subject: [PATCH 40/49] fix: Convert legal hold timestamp to milliseconds for correct date display --- .../legal_hold_table/legal_hold_row/legal_hold_row.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx index adc80cf..e1d57b6 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/legal_hold_row.tsx @@ -29,7 +29,12 @@ const getLastRunDisplay = (lh: LegalHold) => { if (lh.status === 'executing') { return 'Running...'; } - return lh.last_execution_ended_at ? new Date(lh.last_execution_ended_at).toLocaleString() : 'Never'; + if (!lh.last_execution_ended_at || lh.last_execution_ended_at === 0) { + return 'Never'; + } + + // Convert seconds to milliseconds for JavaScript Date + return new Date(lh.last_execution_ended_at * 1000).toLocaleString(); }; const LegalHoldRow = (props: LegalHoldRowProps) => { @@ -57,7 +62,7 @@ const LegalHoldRow = (props: LegalHoldRowProps) => {
- {getLastRunDisplay(lh)} {lh.last_execution_ended_at} + {getLastRunDisplay(lh)}
Date: Wed, 15 Jan 2025 13:01:02 +0100 Subject: [PATCH 41/49] chore: change system admin comment --- .../legal_hold_table/legal_hold_row/run_error_modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx b/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx index 5b90026..b54c687 100644 --- a/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx +++ b/webapp/src/components/legal_hold_table/legal_hold_row/run_error_modal.tsx @@ -17,7 +17,7 @@ const RunErrorModal = ({show, onHide}: Props) => { {'Error Running Legal Hold'} - {'There was an error running the legal hold. Please contact your system administrator.'} + {'There was an error running the legal hold. Please check the logs for more details.'}