Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow running a legal hold manually #129

Open
wants to merge 67 commits into
base: main
Choose a base branch
from
Open

Conversation

fmartingr
Copy link
Contributor

@fmartingr fmartingr commented Jan 13, 2025

This pull request introduces new functionalities to manage and execute individual legal holds, along with corresponding tests and necessary refactoring. The most important changes include adding new API endpoints, implementing methods to run and reset legal holds, and updating the data model to include status tracking.

New API Endpoints:

  • Added routes to run and reset the status of a specific legal hold in server/api.go.

Methods for Legal Hold Management:

  • Implemented the runSingleLegalHold and resetLegalHoldStatus methods in server/api.go to handle the new API endpoints.
  • Added RunSingleLegalHold method in server/jobs/legal_hold_job.go to execute a specific legal hold.
  • Executing a Legal Hold will store the last execution date as the current timestamp if the one being used is set in the future. (Automated jobs vs manual jobs)

Data Model Updates:

  • Introduced LegalHoldStatus and added status tracking fields in server/model/legal_hold.go

Interface and Refactoring:

  • Defined LegalHoldJobInterface to standardize the interface for legal hold jobs, facilitating easier testing and future enhancements in server/jobs/legal_hold_job_interface.go.

Important changes

  • Removed the double loop when executing legal holds. This was used to retry a legal hold but also means that if a legal hold fails for any reason the rest of the holds will not be handled while one fails continuously. (src)

UI Changes

Screenshot 2025-01-13 at 15 56 58

New button to refresh the legal hold list manually

Screenshot 2025-01-13 at 15 57 49

Added a button to run a legal hold manually

Screenshot 2025-01-13 at 15 59 26

Show when a current legal hold is running

Screenshot 2025-01-13 at 15 57 42

Disable the run button if the legal hold is already running and the download button if there's no messages in the legal hold

Enabled Disabled
Screenshot 2025-01-13 at 16 02 13 Screenshot 2025-01-13 at 15 57 20

Closes #90
Fixes #89

feat: Add reset status endpoint with five-click trigger for legal hold

fix: Import Client in legal_hold_row.tsx to resolve ReferenceError
if lh.IsFinished() {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s has ended and therefore will not execute.", lh.ID))
if legalHold.IsFinished() {
j.client.Log.Debug(fmt.Sprintf("Legal Hold %s has ended and therefore does not executing.", legalHold.ID))
Copy link
Member

@wiggin77 wiggin77 Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor grammar issue.


if end, err := lhe.Execute(); err != nil {
if updatedLH, err := lhe.Execute(now); err != nil {
if strings.Contains(err.Error(), "another execution is already running") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is another execution is already running still the error string returned when the mutex cannot be locked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where this comes from. At first I though this was added by aider somehow but I added this myself and it just couldn't work because the cluster mutex does not return an error if the mutex is already locked, it waits for it to be unlocked.

@@ -153,6 +192,11 @@ func (ex *Execution) ExportData() error {
return err
}

// Since at this point we have posts, ensure the `HasMessages` is set to true so users can
// download the legal hold.
ex.papi.LogInfo("has_messages = true")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to be an info level log message? I don't think it has enough context to output at that level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, that was a leftover from me trying to figure out why the legal hold was not being updated.

@fmartingr fmartingr requested a review from wiggin77 February 17, 2025 08:52
@esarafianou esarafianou self-requested a review February 17, 2025 11:40
@esarafianou esarafianou added the 3: Security Review Review requested from Security Team label Feb 17, 2025
@fmartingr fmartingr added the 3: QA Review Requires review by a QA tester label Feb 20, 2025
}

time.Sleep(time.Millisecond * 250)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a leftover of testing of some sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester 3: Security Review Review requested from Security Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a "run now" button on legal holds in system console
6 participants