mattermost / mattermost-plugin-legal-hold

Plugin to create and manage legal holds in Mattermost
Other
5 stars 6 forks source link

Remove compliance check from the code #116

Closed fmartingr closed 1 month ago

fmartingr commented 1 month ago

Summary

Ticket Link

https://mattermost.atlassian.net/browse/MM-59830

Screenshots

Screenshot 2024-09-25 at 18 28 55

Replaces #115 Closes #114

fmartingr commented 1 month ago

@AayushChaudhary0001 This PR is ready for QA.

AayushChaudhary0001 commented 1 month ago

I was testing this PR and had a suggestion on this. The helper text added for the functionality looks a bit confusing, can you please change it and add what this functionality actually is responsible for after enabling and disabling. image

AayushChaudhary0001 commented 1 month ago

On testing this PR, I am facing an error, inspite of changing the compliance monitoring to true or false. Earlier it was due to compliance monitoring but I don't think this time its the same reason. Please refer the screenshot below. image

fmartingr commented 1 month ago

On testing this PR, I am facing an error, inspite of changing the compliance monitoring to true or false. Earlier it was due to compliance monitoring but I don't think this time its the same reason. Please refer the screenshot below. image

Do you have anything in the server logs? If this is a legal hold that you had before this PR, delete it and try again, since the data structure has changed.

AayushChaudhary0001 commented 1 month ago

I am facing an other issue in this PR, the changes done in any legal hold are not getting saved in a single time, and some fields are changed on opening the legal hold creation modal again. Please refer the screenshots below. FYI, no new change is found in the logs.

Existing legal hold image

On updating and saving image

Again updating, when opened the the modal image

fmartingr commented 1 month ago

Hey @AayushChaudhary0001, thanks for catching that. I just pushed a fix, I'm not experiencing that behavior anymore:

https://github.com/user-attachments/assets/c5d32e80-0ff4-4b9e-8585-a721c883b4c7

I also fixed the Update/Show secret tooltips that I saw they were switched out.

fmartingr commented 1 month ago

Hey @AayushChaudhary0001 I just saw you 👍 my comment. Did you test that on your end again and it was fixed and QA passing?

AayushChaudhary0001 commented 1 month ago

@fmartingr Will let you know soon, currently under QA testing.

AayushChaudhary0001 commented 1 month ago

@fmartingr The fix has been tested and works well. The compliance check has also been removed, user is able to include and exclude the public channels without any dependency of compliance monitoring. But the size of ZIP file getting downloaded on enabling or disabling the including public channels functionality seems to be similar, can you please take a look at it?

fmartingr commented 1 month ago

@fmartingr The fix has been tested and works well. The compliance check has also been removed, user is able to include and exclude the public channels without any dependency of compliance monitoring. But the size of ZIP file getting downloaded on enabling or disabling the including public channels functionality seems to be similar, can you please take a look at it?

Hey @AayushChaudhary0001, I just checked two legal holds for the same users and same dates and one of them got the public channels and the other didn't. Did you check the content of the files or the output with the processor?

AayushChaudhary0001 commented 1 month ago

@fmartingr On editing the config, I am facing this error, Can you please have a look at it why is this happening? I am attaching a screenshot and server logs as well.

image

{"timestamp":"2024-10-03 18:25:59.942 +05:30","level":"error","msg":"could not update legal hold as it has already been updated by someone else","caller":"app/plugin_api.go:1003","plugin_id":"com.mattermost.plugin-legal-hold"}

I am testing this on my local instance and there is no other user present here.

fmartingr commented 1 month ago

@fmartingr On editing the config, I am facing this error, Can you please have a look at it why is this happening? I am attaching a screenshot and server logs as well.

image

{"timestamp":"2024-10-03 18:25:59.942 +05:30","level":"error","msg":"could not update legal hold as it has already been updated by someone else","caller":"app/plugin_api.go:1003","plugin_id":"com.mattermost.plugin-legal-hold"}

I am testing this on my local instance and there is no other user present here.

@AayushChaudhary0001 Are you testing this exact branch or are you testing this directly from master ? What field did you edit (if any) ?

AayushChaudhary0001 commented 1 month ago

The above issue is not now reproducible

fmartingr commented 1 month ago

The above issue is not now reproducible

👍 Thank you.