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

WorldMap: Dataset citations longer than 255 chars cause an error #3432

Closed
raprasad opened this issue Oct 26, 2016 · 12 comments
Closed

WorldMap: Dataset citations longer than 255 chars cause an error #3432

raprasad opened this issue Oct 26, 2016 · 12 comments

Comments

@raprasad
Copy link
Contributor

raprasad commented Oct 26, 2016

Problem

  • The code limits the Dataset citation to 255 characters—the error is happening when the citation is longer

First impression

  • Full Fix:
    1. Change the “geoconnect” code to accept a longer citation (nearly unlimited)
    2. Change the “geoconnect" database column type to accept a longer citation
      1. Change the WorldMap code (shared) to accept a longer citation
      2. Change the WorldMap database column type to accept a longer citation
  • Partial fix:
    • Do 1. and 2. which doesn’t touch the WorldMap
    • Pros: user should see be able to run map and see expected results
    • Cons: Citation stored in WorldMap will be truncated

image001

@raprasad
Copy link
Contributor Author

raprasad commented Apr 5, 2017

primary code change

File to change: https://github.com/IQSS/shared-dataverse-information/blob/master/shared_dataverse_information/dataverse_info/models.py

  1. Change dataset_citation:
    • Original: dataset_citation = models.CharField(max_length=255)
    • Updated: dataset_citation = models.TextField()
  2. Add a test, including a non-English language test
  3. Update the changelog, version, etc within the library

steps after change is made and checked in:

shared dataverse information

Geoconnect

WorldMap

Note 1: WorldMap should be updated before geoconnect on dev/prod

Note 2: Checked the 3 codebases* that dataset_citation size doesn't appear on any validation forms, etc. e.g., the model size (currently 255) is used for any validation. * (shared_dataverse_information, geoconnect, worldmap)

@pdurbin pdurbin added in progress and removed ready labels Apr 11, 2017
@pdurbin pdurbin self-assigned this Apr 11, 2017
@pdurbin
Copy link
Member

pdurbin commented Apr 11, 2017

We discussed this issue at standup this morning and I floated the idea of doing the "partial fix" described above of truncating the citation on the WorldMap side. It sounds like we don't want this so it would be a waste of effort. I guess I'll leave this issue assigned to me for now because I'd like to see if I can add a failing test.

Speaking of tests, I also added tests as part of #3489 but they are not automatically executed so they could start failing at any time. Since we're already using https://travis-ci.org/IQSS for Dataverse and Zelig, we should probably attempt to figure out how to add a Django app to Travis and run the tests automatically. I did a quick search for this but it seems somewhat involved: https://docs.travis-ci.com/user/languages/python/

@pdurbin
Copy link
Member

pdurbin commented Apr 12, 2017

I'm blocked on this issue because I can't get WorldMap installed so I'm unassigning myself. The error I'm seeing is at the paver build step: IQSS/worldmap-legacy-dev@fc81368#commitcomment-21735918

@pdurbin pdurbin assigned pdurbin and unassigned pdurbin Apr 12, 2017
@pdurbin pdurbin added ready and removed in progress labels Apr 12, 2017
@pdurbin pdurbin removed their assignment Apr 12, 2017
@raprasad raprasad self-assigned this Apr 13, 2017
raprasad added a commit to IQSS/shared-dataverse-information that referenced this issue Apr 14, 2017
…eanField() - Changed field in DataverseInfo: dataset_citation was CharField, now TextField

IQSS/dataverse#3432
@raprasad
Copy link
Contributor Author

raprasad commented Apr 14, 2017

  • dataverse codebase update to pass datafile.is_restricted value
  • shared dataverse information repository update
  • geoconnect repository update
    • Run: heroku run 'python manage.py migrate --settings=geoconnect.settings.heroku'
    • geoconnect dev db update (updated requirements)
    • geoconnect prod db update (updated requirements)
  • worldmap repository update
    • worldmap prod db update (updated requirements)
      • May require manual db updates!

raprasad added a commit to IQSS/geoconnect that referenced this issue Apr 14, 2017
@raprasad
Copy link
Contributor Author

Code updated on 2/4 repositories. and 2/4 servers

raprasad added a commit that referenced this issue Apr 17, 2017
@raprasad
Copy link
Contributor Author

Last commit is a 1-liner for the dataverse codebase

raprasad added a commit to cga-harvard/geonode that referenced this issue Apr 17, 2017
@raprasad
Copy link
Contributor Author

pull requested add to worldmap codebase: cga-harvard/geonode#228

@raprasad
Copy link
Contributor Author

Testing

  • Create a dataset where the citation is over 255 characters
  • Add a mappable file (e.g. shapefile)
  • Check if the citation is carried to the final map. Currently, WorldMap returns a non-descript error b/c of a 255 char limit on the citation size

@raprasad raprasad removed their assignment Apr 17, 2017
@raprasad
Copy link
Contributor Author

raprasad commented Apr 17, 2017

Status:

@pdurbin
Copy link
Member

pdurbin commented Apr 18, 2017

It looks like IQSS/geoconnect#129 was already merged. cga-harvard/geonode#228 hasn't but looks fine. #3772 is small and looks good. I'm moving this to QA at https://waffle.io/IQSS/dataverse

@pdurbin pdurbin removed their assignment Apr 18, 2017
@raprasad
Copy link
Contributor Author

@kcondon will you let know when WorldMap code has been deployed for testing

@raprasad
Copy link
Contributor Author

How to test (from comment above):

  • Create a dataset where the citation is over 255 characters
  • Add a mappable file (e.g. shapefile)
  • Check if the citation is carried to the final map. Currently, WorldMap returns a non-descript error b/c of a 255 char limit on the citation size

@kcondon kcondon self-assigned this Apr 18, 2017
kcondon added a commit that referenced this issue Apr 18, 2017
#3432 : WorldMap: Dataset citations longer than 255 chars cause an error
@kcondon kcondon closed this as completed Apr 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants