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

Embargo Compliance - Persist Checkbox in DB for ICLA/ECLA #4482

Open
mlehotskylf opened this issue Nov 20, 2024 · 12 comments
Open

Embargo Compliance - Persist Checkbox in DB for ICLA/ECLA #4482

mlehotskylf opened this issue Nov 20, 2024 · 12 comments
Assignees

Comments

@mlehotskylf
Copy link
Contributor

mlehotskylf commented Nov 20, 2024

When user confirm Embargo Compliance (select checkbox), this confirmation information must be persisted in the database.

What needs to be done:
We need to either modify the existing API or implement a new API to save the value of the “compliance” checkbox to the database (I suggest using the signatures table). This may require adding a new column or field to the signatures table in DynamoDB to store this value. Once a user or organization acknowledges compliance, we will save TRUE in this field. Existing records should remain unchanged, with this field left as NULL.

More details in https://docs.google.com/document/d/1vWgeGsvnJq5IMIr4ZNwTTPrll4_2ros8VA2GfhTufFg/edit?tab=t.0#heading=h.a5yu2hojiwh7

Related UI ticket: #4483

Image

@lukaszgryglicki
Copy link
Contributor

On it.

@mlehotskylf mlehotskylf changed the title Implement Embargo Compliance - Milestone 1 Embargo Compliance - Persist Checkbox in DB for ICLA/ECLA Nov 20, 2024
@lukaszgryglicki
Copy link
Contributor

Resolved all issues with AWS access for DynamoDB and MFA setup. I'm now able to access EasyCLA DynamoDB both via AWS Web/UI and via aws command line tools (using MFA app).

@lukaszgryglicki
Copy link
Contributor

One of endpoints to update is the python one: /v1/request-employee-signature for ecla.
I try to figure out the golang one too (and need to make sure that there are just those two, not more... - asked on slack).

@lukaszgryglicki
Copy link
Contributor

PR up: #4485 but due to AWS problems I've marked it as do-not-merge - all details on the PR, I'll update here once I resolve my AWS issues.

@lukaszgryglicki
Copy link
Contributor

@mlehotskylf FYI: https://linuxfoundation.slack.com/archives/C0697E1QHNG/p1732700046666529?thread_ts=1732696733.625019&cid=C0697E1QHNG - PTAL, copying here:

Currently I was told to add signature_embargo_acked field to cla-{{stage}}-signatures table when creating a new signature (which means that new records will have that field set to true meaning embargo was acknowledged - as the new UI is showing that checkbox and doesn’t allow to proceed without checking it), if there is an already existing signature it is not changed because it might have been created with an old UI that didn’t had that checkbox. Let me know if specs changed for this task - update the task then.

And please put a list of APIs that should be updated in this ticket (if this is not a big problem) because after talking with
@Harold Wanyama
it seems like I’ve already attempted to update an API that is not really used anymore - maybe we should mark such apis in code as “unused” somehow? I currently got this list:

request-individual-signature (v2)
request-employee-signature (v2)
request-corporate-signature (v4)

1st and 2nd are python and 3rd is golang - is this the complete list? Or maybe I should update more APIs?

@lukaszgryglicki
Copy link
Contributor

I've heard on the call that all those 3 API are using golang which contradicts the previous specs.
Can I have a final list of APIs to update during this task (this is my 1st task and I really have no knowledge about this - so Ineed at leats those specs to be clear and proceed).

Currently I'm blocked on this - my PR was already almost ready for review when I got that info which invalidates it.

cc @mlehotskylf @nickmango

@lukaszgryglicki lukaszgryglicki added the blocked Don't merge label Dec 3, 2024
@lukaszgryglicki
Copy link
Contributor

I don't have this API list yet.

@lukaszgryglicki
Copy link
Contributor

PR is ready for review: #4485 cc @mlehotskylf @nickmango

@lukaszgryglicki
Copy link
Contributor

lukaszgryglicki commented Dec 5, 2024

PR approved & merged, moving this ticket to Ready for review cc @mlehotskylf
LMK what is the next step here - what is the action item for me now.

@thakurveerendras
Copy link
Contributor

@lukaszgryglicki , kindly share the API endpoint (swigger details) for testing on the dev
CC: @mlehotskylf
Thanks

@lukaszgryglicki
Copy link
Contributor

lukaszgryglicki commented Dec 6, 2024

It is supposed to be persisted for all flows (ICLA, ECLA, CCLA) - not a specific endpoint, so this requires testing of all possible flow types.

@thakurveerendras
Copy link
Contributor

@amolsontakke3576 , Kindly share the contributor console branch on which I can verify UI changes
Refer to the screenshot
image

@lukaszgryglicki lukaszgryglicki removed the blocked Don't merge label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants