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

Fix #10629 - Duplicating Surveys #9498

Closed
wants to merge 3 commits into from

Conversation

SinergiaCRM
Copy link
Contributor

@SinergiaCRM SinergiaCRM commented Feb 7, 2022

Closes #8947

As the Issue mentions, there is a problem when duplicating Surveys. Basically, what goes wrong when duplicating:

  • If the base survey has questions and no answers, the duplicated survey will take all the questions, leaving the base survey without questions
  • If the base survey has questions and answers, the duplicated survey won't take/keep the questions, neither the answers. Although the Editview that appears after duplicating will contain the questions and won't allow it to modify them:
    Selection_430

Closes #10629

Having a WorkFlow and duplicating a survey or create also duplicated the questions, as the code was executed without any workflow verification. This was because:

  • When duplicating survey X to Y or creating survey X, having WorkFlow active, the code saved the questions normally.
  • But because there was no control over the workflow, when the workflow was executed:
    • The code re-executed the foreach loop that saves the questions.
    • As a result, the questions were saved twice: once during the normal save and once during the workflow execution.

Description

This PR fixes the Duplicate functionality for the Surveys module, as we understand it should work:

  • If the base survey has questions and no answers, the duplicated survey will keep the questions, duplicating them.
  • If the base survey has questions and answers, the duplicated survey won't will take/keep/duplicate the questions or but not the answers. And other new Questions can be added during the first edition after duplication.
  • Fixed the workflow problem by adding a control variable already_saved which is used to verify that the method is not re-executed. If already_saved is set, it means that the method has already been executed and reprocessing of the questions is avoided.

Motivation and Context

With this change, duplicating Surveys will be consistent.

How To Test This

1- Create a Survey
2- Add questions
3- Duplicate it.
4- Repeat adding answers to the survey.

  • Workflow Issue:
  1. WorkFlow Module: Surveys
  2. Run: Only On Save
  3. Run On: New Records
  4. Action:
    4.1. Modify the survey itself, for example, change from draft to published status.
    4.2. Calculate fields, for example, calculate the name.
  5. Duplicate the survey from step 1, checking the number of questions it has.
  6. Check that the survey has been duplicated without duplicating the questions and that it has the same questions as the survey from step 1.
  7. Create a survey with several questions and check that the surveys are not duplicated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • My code follows the code style of this project found here.
  • My change requires a change to the documentation.
  • I have read the How to Contribute guidelines.

@jack7anderson7
Copy link
Contributor

Hi @SinergiaCRM,

The fix looks good, there's just one change that would be ideal. When duplicating surveys it is preferred if the questions are always carried across even when they have answers, just the answers wouldn't.

Thanks,
Jack

@jack7anderson7 jack7anderson7 added Branch:Hotfix Status: Requires Updates Issues & PRs which requires input or update from the author labels Feb 11, 2022
@SinergiaCRM
Copy link
Contributor Author

Hi @SinergiaCRM,

The fix looks good, there's just one change that would be ideal. When duplicating surveys it is preferred if the questions are always carried across even when they have answers, just the answers wouldn't.

Thanks, Jack

Hi @jack7anderson7,

Yes, this was actually what we wanted to implement. Updated the main PR message and the code. 👍

@jack7anderson7 jack7anderson7 added Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member and removed Status: Requires Updates Issues & PRs which requires input or update from the author labels Feb 14, 2022
jack7anderson7
jack7anderson7 previously approved these changes Feb 14, 2022
@SinergiaCRM
Copy link
Contributor Author

Updated the code, there was a syntax error

Copy link

@phil8900 phil8900 left a comment

Choose a reason for hiding this comment

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

Fix is working and should be merged I think. @jack7anderson7 could you take a look at this? :)

@serhiisamko091184 serhiisamko091184 added the Area:Surveys Issues & PRs related to all things regarding Surveys label Sep 21, 2023
@serhiisamko091184
Copy link
Contributor

Hello @SinergiaCRM ,
thanks a lot for your contribution,

would you be so kind to combine commits into one?

Thank you in advance!

Regards,
Serhii

@serhiisamko091184 serhiisamko091184 added Status: Requires Updates Issues & PRs which requires input or update from the author and removed Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member labels Sep 29, 2023
@SinergiaCRM
Copy link
Contributor Author

SinergiaCRM commented Jan 23, 2024

Hello @SinergiaCRM , thanks a lot for your contribution,

would you be so kind to combine commits into one?

Thank you in advance!

Regards, Serhii

Hi @serhiisamko091184

Done!

@SinergiaCRM SinergiaCRM requested a review from phil8900 January 29, 2024 08:28
@serhiisamko091184 serhiisamko091184 added Status: Requires Code Review Needs the core team to code review PR 4-8 Complexity Score given to PRs once assessed Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member and removed Status: Requires Updates Issues & PRs which requires input or update from the author labels Mar 18, 2024
@serhiisamko091184
Copy link
Contributor

Hello @SinergiaCRM,

great, many thanks!

Please update your branch later on:

image

Regards,
Serhii

@SuiteBot
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ SinergiaCRM
❌ AlbertoSTIC
You have signed the CLA already but the status is still pending? Let us recheck it.

@SinergiaCRM SinergiaCRM changed the title Fix #8947 - Duplicating Surveys Fix #10629 - Duplicating Surveys Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Surveys Issues & PRs related to all things regarding Surveys Branch:Hotfix PR 4-8 Complexity Score given to PRs once assessed Status:Assessed PRs that have been tested and confirmed to resolve an issue by a core team member Status: Requires Code Review Needs the core team to code review
Projects
None yet
6 participants