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: 500 error on Dataset validation #246

Merged
merged 3 commits into from
May 11, 2021
Merged

Conversation

Geeker1
Copy link
Contributor

@Geeker1 Geeker1 commented Apr 13, 2021

Description

During dataset upload, an Attribute error is thrown if profile is None, fixed this by writing the clean method for that field, raising a Validation error if profile is None.

Related Issue

#204

How to test it locally

Changelog

Added

Updated

Removed

Checklist

  • 🚀 is the code ready to be merged and go live?
  • 🛠 does it work (build) locally

Pull Request

  • 📰 good title
  • 📝good description
  • 🔖 issue linked
  • 📖 changelog filled out

Commits

  • commits are clean
  • commit messages are clean

Code Quality

  • 🚧 no commented out code
  • 🖨 no unnecessary logging
  • 🎱 no magic numbers
  • black was run locally (as part of the pre-commit hook)

Testing

  • ✅ added (appropriate) unit tests
  • 💢 edge cases in tests were considered
  • ✅ ran tests locally & are passing

@milafrerichs milafrerichs temporarily deployed to wazimap-error-validatio-iw33fl April 13, 2021 07:02 Inactive
@Geeker1 Geeker1 marked this pull request as ready for review April 13, 2021 07:07
@Geeker1 Geeker1 requested a review from milafrerichs April 13, 2021 07:11
Copy link
Contributor

@milafrerichs milafrerichs left a comment

Choose a reason for hiding this comment

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

this not the intended solution.

the profile is not mandatory. but the profile is mandatory when the permission type is private. so we need a check here but only if the permission is private.

What we need to do is to check the code here:

https://github.com/OpenUpSA/wazimap-ng/blob/staging/wazimap_ng/datasets/admin/dataset_admin.py#L113-L114

And only run that if the profile is set.

profile = cleaned_data.get('profile', None)
permission_type = cleaned_data.get('permission_type', None)

if not profile:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would merge those together and make it more obvious
if permission_type == 'private' and profile == None

@milafrerichs milafrerichs merged commit 5bbf7b9 into staging May 11, 2021
@adieyal adieyal deleted the error-validation-204 branch August 6, 2021 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants