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: cannot save nullish values for required fields #2003

Conversation

RaschidJFR
Copy link
Contributor

@RaschidJFR RaschidJFR commented Jan 16, 2022

Fixes #1980

New Pull Request Checklist

Issue Description

Related issue: #1980

Approach

Instead of checking for value being nullish (eg !value), check for it being different from null and from empty string ('')

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jan 16, 2022

Thanks for opening this pull request!

  • ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

@RaschidJFR
Copy link
Contributor Author

I'm having trouble creating the tests for BrowserCell. Jest failed for imports using double quotes (") instead of single quotes ('). I tried a couple of things by changing the regex in preprocessor.js, but no luck.

So I proceeded to lint-fix all files... should I do it in this PR?

@RaschidJFR RaschidJFR changed the title fix(Browser): should allow saving nullish values fix: should allow saving nullish values Jan 16, 2022
@RaschidJFR RaschidJFR changed the title fix: should allow saving nullish values fix: should allow saving nullish values for required fields Jan 16, 2022
@mtrezza
Copy link
Member

mtrezza commented Jan 17, 2022

So I proceeded to lint-fix all files... should I do it in this PR?

Uh, please in an extra PR, to make it easier to review the 2 PRs.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! Is this ready for merge?

@RaschidJFR
Copy link
Contributor Author

It is ready, but for the record, when the changes in #2004 are applied, the test here created will fail:

 FAIL  src/lib/tests/BrowserCell.test.js

  ● Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /Users/raschidjfr/Documents/git/github/parse-community/parse-dashboard/node_modules/react-popper-tooltip/dist/styles.css:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){.tooltip-container {
                                                                                             ^

    SyntaxError: Unexpected token '.'

      1 | import React from 'react';
      2 | import { usePopperTooltip } from 'react-popper-tooltip';
    > 3 | import 'react-popper-tooltip/dist/styles.css';
        | ^
      4 |
      5 | const PopperTooltip = (props) => {
      6 |   const { children, tooltip, visible, placement } = props;

      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
      at Object.<anonymous> (src/components/Tooltip/PopperTooltip.react.js:3:1)

Test Suites: 1 failed, 15 passed, 16 total
Tests:       61 passed, 61 total
Snapshots:   0 total
Time:        5.623s
Ran all test suites.

As the failure is not directly related to this PR, I think it would be a good idea to merge this now and address the problem in #2004 .

@mtrezza
Copy link
Member

mtrezza commented Jan 18, 2022

Sounds good

@mtrezza mtrezza changed the title fix: should allow saving nullish values for required fields fix: cannot save nullish values for required fields Jan 18, 2022
@mtrezza mtrezza merged commit e1a5497 into parse-community:alpha Jan 18, 2022
parseplatformorg pushed a commit that referenced this pull request Jan 18, 2022
# [4.0.0-alpha.9](4.0.0-alpha.8...4.0.0-alpha.9) (2022-01-18)

### Bug Fixes

* cannot save nullish values for required fields ([#2003](#2003)) ([e1a5497](e1a5497))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.0-alpha.9

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jan 18, 2022
@RaschidJFR RaschidJFR deleted the bugfix/cant_save_nullish_required_field branch January 20, 2022 05:15
parseplatformorg pushed a commit that referenced this pull request Feb 6, 2022
# [4.0.0-beta.3](4.0.0-beta.2...4.0.0-beta.3) (2022-02-06)

### Bug Fixes

* bump follow-redirects from 1.14.4 to 1.14.7 ([#1997](#1997)) ([4ca2e97](4ca2e97))
* bump markdown-it from 12.3.0 to 12.3.2 ([#1996](#1996)) ([245c22e](245c22e))
* bump marked from 0.8.2 to 4.0.10 ([#2001](#2001)) ([ae4cc90](ae4cc90))
* bump nanoid from 3.1.28 to 3.2.0 ([#2008](#2008)) ([6cfe9ca](6cfe9ca))
* calendar widget layout partly hides last days of a month ([#1990](#1990)) ([5bd86dd](5bd86dd))
* cannot save nullish values for required fields ([#2003](#2003)) ([e1a5497](e1a5497))
* crash when checking for new dashboard release without internet connection ([#2015](#2015)) ([8c36e69](8c36e69))
* preserve column sorting preferences in data browser ([#2016](#2016)) ([c2e6557](c2e6557))
* upgrade parse from 3.4.0 to 3.4.1 ([#2011](#2011)) ([68cf9e2](68cf9e2))
* various UI bugs (e.g. filter data browser, switch app, upload file) ([#2010](#2010)) ([a508a58](a508a58))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.0-beta.3

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Feb 6, 2022
parseplatformorg pushed a commit that referenced this pull request Mar 3, 2022
## [4.0.1](4.0.0...4.0.1) (2022-03-03)

### Bug Fixes

* bump follow-redirects from 1.14.4 to 1.14.7 ([#1997](#1997)) ([4ca2e97](4ca2e97))
* bump markdown-it from 12.3.0 to 12.3.2 ([#1996](#1996)) ([245c22e](245c22e))
* bump marked from 0.8.2 to 4.0.10 ([#2001](#2001)) ([ae4cc90](ae4cc90))
* bump nanoid from 3.1.28 to 3.2.0 ([#2008](#2008)) ([6cfe9ca](6cfe9ca))
* calendar widget layout partly hides last days of a month ([#1990](#1990)) ([5bd86dd](5bd86dd))
* cannot save nullish values for required fields ([#2003](#2003)) ([e1a5497](e1a5497))
* crash when checking for new dashboard release without internet connection ([#2015](#2015)) ([8c36e69](8c36e69))
* preserve column sorting preferences in data browser ([#2016](#2016)) ([c2e6557](c2e6557))
* upgrade parse from 3.4.0 to 3.4.1 ([#2011](#2011)) ([68cf9e2](68cf9e2))
* various UI bugs (e.g. filter data browser, switch app, upload file) ([#2010](#2010)) ([a508a58](a508a58))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.1

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot save required field with value 0
3 participants