Skip to content

Do not migrate surveys that were already migrated in the past#6380

Merged
tramuntanal merged 1 commit intodevelopfrom
fix/do_not_migrate_survey_if_already_migrated
Aug 11, 2020
Merged

Do not migrate surveys that were already migrated in the past#6380
tramuntanal merged 1 commit intodevelopfrom
fix/do_not_migrate_survey_if_already_migrated

Conversation

@tramuntanal
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal commented Aug 4, 2020

🎩 What? Why?

In #6299 I forgot to modify the code in the rake task to check if a survey was already migrated. In these cases another (empty) Questionnaire is associated to the survey. Then this empty Questionnaire uses to be the one resolved by survey.questionnaire and it renders empty in the backoffice and in the public views.

This PR adds a check before migrating each survey and if survey.questionnaire.present? the survey is not migrated.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG upgrade notes, if required
  • If there's a new public field, add it to GraphQL API
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests
  • Another subtask

📷 Screenshots (optional)

Description

@carolromero carolromero mentioned this pull request Aug 4, 2020
Closed
13 tasks
Leusev
Leusev suggested changes Aug 6, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Leusev Leusev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @tramuntanal
Maybe you should review the check of the table_exists method call for up method:

image

as I've seen the comment by @andreslucena on closed previous PR #6299:

where he asked about check if table data exists instead of only table exists.
He linked into issue #6275 description as follows:

image

@tramuntanal
Copy link
Copy Markdown
Contributor Author

tramuntanal commented Aug 6, 2020

@ramoncodit the ... if Question.any? checks if there is still content in the legacy tables. This PR adds a check inside the migrate_legacy_data to not migrate again if a Questionnaire already exists for the survey.

In the legacy structure survey -> Decidim::Surveys::Question, with the replacing decidim-forms, survey -> questionnnaire -> Decidim::Forms::Question

@Leusev Leusev self-requested a review August 10, 2020 07:04
Leusev
Leusev approved these changes Aug 10, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Leusev Leusev left a comment

Choose a reason for hiding this comment

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

Ok, then the asked fix is contemplated. All ok 👍

@tramuntanal tramuntanal merged commit 4b8b026 into develop Aug 11, 2020
@tramuntanal tramuntanal deleted the fix/do_not_migrate_survey_if_already_migrated branch August 11, 2020 11:11
@tramuntanal tramuntanal mentioned this pull request Aug 11, 2020
Merged
6 tasks
Quentinchampenois pushed a commit to Quentinchampenois/decidim that referenced this pull request Aug 14, 2020
@tramuntanal @Quentinchampenois
Do not migrate surveys that were already migrated in the past (decidi…
45a4a2c 
…m#6380)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@ivan-mr ivan-mr ivan-mr approved these changes

@Leusev Leusev Leusev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review module: surveys

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tramuntanal @ivan-mr @Leusev