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: don't send param with no initialValue #7647

Merged
merged 6 commits into from
Sep 30, 2021

Conversation

rishabhrathod01
Copy link
Contributor

@rishabhrathod01 rishabhrathod01 commented Sep 20, 2021

Description

Fixes #7648

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Test coverage results 🧪

⚪ Total coverage has not changed No changes to code coverage between the base branch and the head branch

@github-actions github-actions bot added the Bug Something isn't working label Sep 20, 2021
@rishabhrathod01 rishabhrathod01 self-assigned this Sep 20, 2021
@rishabhrathod01 rishabhrathod01 added the Generate Page Issures related to page generation label Sep 20, 2021
@mohanarpit
Copy link
Member

@Rishabh-Rathod Is this PR still required after the fix in #7637 ?

@rishabhrathod01
Copy link
Contributor Author

@Rishabh-Rathod Is this PR still required after the fix in #7637 ?

Yes, this will avoid generate CRUD feature to fail for params not having initialValue.

rishabhsaxena
rishabhsaxena previously approved these changes Sep 21, 2021
@rishabhrathod01 rishabhrathod01 enabled auto-merge (squash) September 21, 2021 10:48
@Nikhil-Nandagopal
Copy link
Contributor

@Rishabh-Rathod can we merge this in? This seems like a big issue

@rishabhrathod01
Copy link
Contributor Author

rishabhrathod01 commented Sep 22, 2021

@Rishabh-Rathod can we merge this in? This seems like a big issue

This issue isn't in the current release, only once the frontend changes in this PR go to release we would have the issue,
which is solved by the backend changes of this PR itself.

This PR is mainly to make generate page features not break in the future due to changes like adding a new param without initialValue.
@Nikhil-Nandagopal

@rishabhrathod01 rishabhrathod01 merged commit 70e6e54 into release Sep 30, 2021
@rishabhrathod01 rishabhrathod01 deleted the fix/crud-gsheets-error branch September 30, 2021 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Generate Page Issures related to page generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Error sending config params with invalid value ( empty string )
6 participants