ericvaandering/DocDB

don't allow preserve sign option for changing docs or adding files ...

Closed this issue · 43 comments

if the user is only in the group to allow changing meta-data with preserving signatures.

If I am in the group that is allowed preserve only on changing meta-data,
but not on changing the document and not on adding files -
(i.e. I am in the group in @HackPreserveSignoffGroups,
but not in the group in @HackDocsPreserveSignoffGroups) -

then I should not be given the message or checkbox to preserve data
if I go to change the document or add files. Here's the message:
(which is correct for changing meta-data, but not for changing the doc or adding files)

Warning: You are about to clear all the signatures on the document,
unless you check the box (near the bottom of this page) to preserve signatures.

and the preserve checkbox does appear at the bottom of the page.

If I am in the needed @hackdocspreservesignoffgroups,
but not in @hackpreservesignoffgroups, I don't get the
option to preserve on a metadata change.

i.e. I can preserve signatures when I change a document
or add files, but not when I change meta-data.

This may be a feature in that you can control them separately
and we can just tell people they need to be in both hack
groups to be able to preserve signoff while changing
metadata, documents and adding files.

So I am fine with this. Just letting you know what it does
since it sounded like you may have thought otherwise earlier.

Other problem: If I am in the @hackdocspreservesignoffgroups, and
I go to change a document, it gives me the right message that
I can preserve signatures if I check the preserve check box
and it shows the preserve checkbox.

However when I change the document and check the preserve
checkbox and submit, I get the error:
"There were fatal errors processing your request:
Signatures may only be preserved when modifying document meta-data."
and the document is not changed.

Interestingly, the same thing works perfectly when I go to add files
to the document, which we put in the same category as changing
the document (as opposed to changing meta-data).

I am able to check preserve signoffs and the files are added,
the signatures are not cleared, no emails are sent to signers to approve.
If I choose not to check the preserve signoff box for add files,
that also works as expected, adding the files and clearing the signatures.

FYI, the setting for add files and change metadata is one thing and the document update another:

https://github.com/ericvaandering/DocDB/blob/8_8_9_br/DocDB/cgi/DocDBGlobals.pm#L145

I'll take another look at what you're seeing and sort out the issues.

Hi. Yes, that is the way it works in production now.
A long while back you proposed (and I agreed):

On 12/27/17 11:16 AM, Eric W Vaandering wrote:

On this one issue of preserving signatures. It looks to me like the current behavior allows a person with the permissions to preserve signatures to also add files without removing the signatures. If we are splitting this into two permissions, it seems to me that it makes more sense to put the adding files permission with changing the document, not changing the metadata. What do you think?

Laura: A) If a person can change actual docs without triggering signatures,
they should also be able to change meta-data without triggering signatures.

Eric: Only A makes sense. I don’t really have a way to check that on a full doc
replacement, the meta-data remained constant.

On Dec 5, 2017, at 4:14 PM, Laura M. Mengel lauram@fnal.gov wrote:
Yes, only one checkbox whether to preserve signatures on a form.

  • If changing meta-data, check if they are in one of the groups in
    HackPreserveSignoffGroups or HackDocsPreserveSignoffGroups

  • If changing the document itself [or adding files], check if they
    are in one of the groups in HackDocsPreserveSignoffGroups
    (or whatever you decide to call it)

@lauramengel I found a logic error that probably explains all of this. I fixed it. I also saw that the preserve box would appear on documents with no signatures requested which would just confuse things, so I think I changed that behavior too. Go ahead and test this too.

Since we're getting close I did another rebase, so the FNAL_SSO branch has everything but you'll need to do a reset to get it. Double check that the switching ordered text field still works (#54)

Still seeing some issues:

  1. same as before:
    If I am in the @hackdocspreservesignoffgroups, and
    I go to change a document, it gives me the right message that
    I can preserve signatures if I check the preserve check box
    and it shows the preserve checkbox.

However when I change the document and check the preserve
checkbox and submit, I get the error:
"There were fatal errors processing your request:
Signatures may only be preserved when modifying document meta-data."
and the document is not changed.

  1. When I click on "Add Files", I get Software Error:
    Undefined subroutine &main::RevisionStatus called at
    /web/sites/d/docdbdev.fnal.gov/cgi-bin/sites/DUNE_SSO/AddFilesForm line 102.

These items were fixed and passed tests

  • No preserve signoffs text or checkbox if doc does not have signatures
  • option not given to preserve signoffs for changing a doc if user is only in group to change metadata
  • #54 "text with order" change is working
  • #74 "don't show public link if not public is working
  1. The rare case where a fix is removing code. The prohibition on any modification except meta data was still there. I added a note saying the permissions depend on the type of modification.

  2. Just a missing "require"

Both fixed by #82.

Again, I've rebased FNAL_sso onto the latest fixes.

prior issues re-tested and working except:
when preserve signoffs upon changing a document,
the new version of the document displays as an unmanaged document.

i.e. No "Signoffs:" heading listed near bottom.
(and therefor, no signatures listed, no "remove signature" button displayed
for me as I had approved the prior version.) Signatures were preserved
on the prior version, but not copied to the new version made due to changing a doc.

Ok, I think I found the problem. The old document was not really being looked up. I also added some debugging.

Try again using the branch "fix_sigcopy". Hopefully that works. If not, please send me the debug messages.

When using fix_sigcopy, if I try to create a document from 1 local file on my computer, I get:
(I tried with both cert version and SSO version.

Software error: Undefined subroutine &main::RevisionStatus called at
/web/sites/d/docdbdev.fnal.gov/cgi-bin/sites/DUNE_Cert_Req/DocumentAddForm line 259.

Pull and try again. I had to move a require statement.

I am able to create a doc from a local file successfully now.

Still same issue changing a document with preserved signatures.
The signatures are preserved on the initial version,
but on the new version docb page, the "Signoffs"
heading is not there (nor the previous signer that already
approved it (with a button for them to remove signature).

Will look for debug info

Results for changing doc with preserve signatures
I started clean by making a new version of a doc with 1 signoff (me) and I approved it.
(using SSO DocDB version and my SSO account).
When I change the document from a.txt to b,txt, some excerpts:
(One of them says "Don't understand method to insert file with key 2")

The document's signatures have been identically preserved. Any changes in the list have been ignored.

You were successful. Your Document ID is DUNE-doc-2935, version 2.
From Database DocID: 2935
From Database DRI: 24499 DI: 2935 V: 1
Finding EmailUserID by FNAL SSO name lauram@fnal.gov
Determined user ID to be 6792
User explicity has groups 65 66 67 91
After SSO groups, DocDB groups for user: 65, 66, 67, 91, 65, 66
Final unique DocDB groups for user: 67, 66, 91, 65

From Database DRI: DI: 2935 V: 2
Copying signatures from DRI to 24502 with T/F 1
Adding files for DRI 24502
Trying to upload 1 b.txt Main: 1
Don't understand method to insert file with key 2
From Database DocID: 2935
From Database DRI: 24502 DI: 2935 V: 2

My copy of docdb-fixcopy branch from a pull at 4:09.
So I think I got your most recent?

4:03 on my end. The problem is still this:

Copying signatures from DRI to 24502 with T/F 1

There should be a number after DRI. I've got an idea.

confirmed it's the latest because if I pull again it says "already up to date"

OK. Give it one more(?) try.

ok, starting ...

Do I need to start from scratch or can I just do a git pull?

Just a pull.

Yes, that worked!!
Did same as before (made new clean doc with 1 signature and approved.
and the changed the document with preserve.

Now the signature/approval is preserved on both v1 and v2 and both
have a button the remove signature.

Now debugging excerpts say:
(Is this msg a concern? "Don't understand method to insert file with key 2")

From Database DocID: 2937
From Database DRI: 24504 DI: 2937 V: 1
...
After limiting, user belongs to unique groups 67, 66, 91, 65
Set OldRevID to 24504 in update mode
Copying signatures from DRI 24504 to 24507 with T/F 1
Adding a signoff to DRI 24507
Added a signoff: 1344
Copying a signature for 6792 to SI 1344: T/F 1 at 2018-06-18 16:44:31 
Transferring dependency 1344: to 1344:0
Adding files for DRI 24507
Trying to upload 1 b.txt Main: 1
Don't understand method to insert file with key 2
From Database DocID: 2937
From Database DRI: 24507 DI: 2937 V: 2
Finding EmailUserID by FNAL SSO name lauram@fnal.gov
Determined user ID to be 6792

Is there any reason I need to test this separately for CERT vs SSO?
(Using Cert DocDB vs using SSO DocDB, also using cert account
vs using SSO account - the permutations get mind boggling)

Is the code the same?

Also, might there be a different if the doc had multiple signatures instead of just 1
and/or if the document was not approved (or does it just copy whatever is there)?

Thanks!

Code is the same. I'm going to rearrange the commits so that this is included in the latest FNAL_SSO branch. Will be done before I head home tonight.

I'm guessing the file method warning is harmless. I think it comes from the 2nd file box on the page that you are probably leaving empty. I had to look it up.

It copies whatever is there and the same code is used for copying signatures from one version to another without preserving the state and the metadata update, but a test with two won't hurt. :-) There is a second part of the process that is copying the relationships (a before b) of the signatures.

Ok, that's done. You probably want to reset back to the FNAL_SSO branch.

Thanks. Will do. Yes, I left the 2nd file box on the page empty.

If I am in HackDocsPreserveSignoff and I click to change meta-data,
I am not given the option to preserve data.
(check if user in HackDocsPreserveSignOff or HackPreserveSignoff for meta-data preserve?)

If I click change document or add files,
I am given the option the preserve data.

Do you get a warning message? Which one. You should get one of

Warning: You are about to modify a managed document. or
Warning: You are about to clear all the signatures on the document, unless ...

I get:
Warning: You are about to modify a managed document. This will clear all the signatures on the document.

I found a mistake and added some debugging. That should take care of it. Sorry for the mess with this feature.

I'm going to fix the other one too and then prepare a new FNAL_SSO

If you give me 20 min, I think I can finish the whole set and let you know if I see anything else before you cut another.

Maye we should punt on user only in hackdocs not able to
preserve when click on change metadata. I'd rather avoid
anything that's thorny at this point.

If this is the only issue, we could tell users they have to be in
both hack groups to preserve signatures for both meta-data
changes and document/addfiles changes. That is not much
of an inconvenience.

If they are in hackdocs, they'll be able to preserve when
changing documents and doing addfiles. When they change
a document, they'll also be able to change meta-data too as
part of that transaction. (but it's easy enough to add the user
to both hack groups, so they can do all 3.)

If they are just in hack, they'll only get to preserve when they
change meta-data.

ok. nevermind then.

Hi. The rest of the restest succeeded. So only change needed is allowing
user HackDocsPreserveSignoffs to be able to use the Change Meta-Data
link and get option to preserve signoffs.

(besides adding account, add groups button for new SSO user with no groups/ldap/cert.

Notes and questions:

  • I did the last set of tests with 2 signatures on a document and it preserved the order.
    Both signers signed and the documents was approved through the whole process
    (as was correct).

    1. Would it matter if one had signed and one had not? (since copying is copying?)

    The date on each signature is also correct.
    Something I prefer not to change now:
    The "Last Signed" field in the gray doc info box on the left shows the
    time of the first signature instead of the second signature.
    I think this is not a show stopper and the date on the actual signatures is correct,
    so I'd rather leave this alone and note it for next time.

    1. Is the change so that "HackDocsPreserveSignoff allows user to use change meta-data link and be given preserve option" very localized?

i.e. Do I need re-test everything again, or just the metadata case for user in HackDocsPreserveSignoff?

  1. Also, what is the probability that yesterdays and todays changes would affect
  • user in Hack or HackDocs has ability to preserve signoff but decides not to
    check the preserve box on meta-data, document or addfiles change.
  • user who does not have ability to preserve signoff (no preserve signoff checkbox)
    changes meta-data, document or addfiles change.

Do I need to check these again? These should clear signatures
and do everything as before the preserve signoff feature was added.

Please go ahead and make the new version.

Would it matter if one had signed and one had not? (since copying is copying?)

It should not.

The "Last Signed" field in the gray doc info box on the left shows the time of the first signature instead of the second signature. I'd rather leave this alone and note it for next time.

Agreed. Want to open an issue and target 8.8.10?

Is the change so that "HackDocsPreserveSignoff allows user to use change meta-data link and be given preserve option" very localized?

The code is very localized causing a bug in the code to determine if the preserve option is presented and able to be used. I wouldn't test every bit of the matrix, but a few things here and there.

3 [...]

Zero probability I would say.

New tag on FNAL_SSO is ready

Hi. being in hackdocssignoffs and changing metadata worked.
other preservesignoff tests worked too.
So we are in good shape.
Back in an hour.

Reviewed my results and all were successful (had done all tests except when
preserve signoff not available or not chosen, since those have worked each past trial
and you mentioned these should not be affected.)

Closing Issue.

I believe this is our new release.
Rob has been testing as well and found no problems.

Thank you!!

OK. Let me reassemble the last few commits in a nicer way and consolidate down to one branch. We can still fix any last minute errors.