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

Add support for importing taxonomy via CSV. #289

Merged
merged 1 commit into from
Dec 9, 2014
Merged

Conversation

monfresh
Copy link
Member

@monfresh monfresh commented Dec 9, 2014

This allows categories to be created in a tree structure supported by the ancestry gem. To see an example of a valid CSV file that recreates the Open Eligibility taxonomy, look at data/taxonomy.csv

For more details about how taxonomy works, read our Wiki article on taxonomy basics:
https://github.com/codeforamerica/ohana-api/wiki/Taxonomy-basics

Special thanks to @volkanunsal for the feature request and getting this work started.

This allows categories to be created in a tree structure supported by the ancestry gem. To see an example of a valid CSV file that recreates the Open Eligibility taxonomy, look at data/taxonomy.csv

For more details about how taxonomy works, read our Wiki article on taxonomy basics:
https://github.com/codeforamerica/ohana-api/wiki/Taxonomy-basics

Special thanks to @volkanunsal for the feature request and getting this work started.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 030acc8 on taxonomy-import into b733c26 on master.

monfresh added a commit that referenced this pull request Dec 9, 2014
Add support for importing taxonomy via CSV.
@monfresh monfresh merged commit b4b3b6f into master Dec 9, 2014
@monfresh monfresh deleted the taxonomy-import branch December 9, 2014 06:03
@volkanunsal
Copy link
Contributor

Awesome. Just one question. Does parent_id refer to the taxonomy_id or record id? If it's the former, then perhaps something like parent_taxonomy_id might be a better choice to avoid database errors due to bad record id during the import process, since parent_id is implemented by ancestry gem, and I don't know how well it might play with that gem.

@monfresh
Copy link
Member Author

Calling it parent_id versus parent_taxonomy_id will not make any difference during the import. It would only be a semantic difference. parent_id is only used to look up the parent Category in the database. It's not an attribute that gets written. Did you run into database errors due to a bad record id?

@volkanunsal
Copy link
Contributor

I'll let you know soon. The problems I ran into so far were mass-assignment not allowed errors on parent_name and parent_id fields.

@monfresh
Copy link
Member Author

You can ignore those errors. It's just letting you know that parent_name and parent_id were passed in as attributes, but were not actually written because they are not permitted. I could remove those attributes from the hash before passing them on, but the end result would be the same (minus the non-user facing errors in the log).

@monfresh
Copy link
Member Author

Note that you would also see the same mass-assignment warnings regardless of what the column is called, unless you remove them from the hash before passing them on.

@volkanunsal
Copy link
Contributor

Here is an error I've been getting:

Ancestry::AncestryException: No child ancestry for new record. Save record before performing tree operations.

@@ -0,0 +1,11 @@
class CategoryPresenter < Struct.new(:row)
def to_category
return parent_category(row).children.create(row) if row[:parent_id].present?
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where the error originates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Figured out that there was an error in creating the parent category, which was suppressed because it's not created with a bang.

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.

3 participants