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.

Sign up for GitHub

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

Jump to bottom

Questionnaire functionality split #1221

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Questionnaire functionality split #1221

wants to merge 2 commits into from

Conversation

TtheBC01
Copy link
Contributor

@TtheBC01 TtheBC01 commented Apr 4, 2024

Release Notes

JIRA Link

Summary:

This PR splits the on-chain questionnaire functionality into its own, stand-alone contract. This was done in order to keep the size of the ConsentFactory small enough to deploy to public networks.

Intended results:

  • This will require refactoring calls to Questionnaire logic to point to Questionnairers.sol
  • all other behavior (including function interfaces) will remain the same

Testing Notes:

  • Make sure questionnaires still populate in the UI

Pre-Flight Checks

  • Has QA approved this change for dev?
  • Are all unit tests passing?
  • Have you added this description to this week's [release notes]

@TtheBC01 TtheBC01 requested a review from a team as a code owner April 4, 2024 18:01
@TtheBC01 TtheBC01 marked this pull request as draft April 4, 2024 18:02
seansing9
seansing9 previously approved these changes Apr 5, 2024
View reviewed changes
packages/contracts/contracts/recomender/Questionnaires.sol
__AccessControl_init();
_addDomain("snickerdoodle.com");

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to grant caller the DEFAULT_ADMIN_ROLE

Copy link
Contributor

@seansing9 seansing9 Apr 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Oh nvm, Lucas's PR's got it:

https://github.com/SnickerdoodleLabs/protocol/pull/1220/files#diff-72cf84a6238904373cb18578a5382066b829b700d107b13e60ec4c5f19bd0668R29

@lucas-sdl lucas-sdl added the DEMO-04 Code is merged to DEMO-04 Sandbox label Apr 5, 2024
@moahammadalt moahammadalt dismissed seansing9’s stale review April 8, 2024 18:43

The merge-base changed after approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seansing9 seansing9 seansing9 left review comments

@moahammadalt moahammadalt Awaiting requested review from moahammadalt

@okanzdmr okanzdmr Awaiting requested review from okanzdmr

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
DEMO-04 Code is merged to DEMO-04 Sandbox
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TtheBC01 @seansing9 @lucas-sdl