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 values.yml and update seeding to use it #117

Merged
merged 3 commits into from
Mar 25, 2024
Merged

Conversation

KComrade53
Copy link
Contributor

Description

Add a new values.yml file under data/values for storing values and their ids instead of directly under attributes. The serializer and seeding logic has been updated to reflect these changes.

The yaml files under data/ are also updated as part of this PR with the output of our new parser.

Questions for reviewers

  • Do I need to increment the version and add to the changelog as part of this PR?

@KComrade53 KComrade53 self-assigned this Mar 21, 2024
app/serializers/data/property_serializer.rb Outdated Show resolved Hide resolved
db/schema.rb Show resolved Hide resolved
db/seed.rb Outdated Show resolved Hide resolved
db/seed.rb Outdated Show resolved Hide resolved
test/integration/all_data_files_import_test.rb Outdated Show resolved Hide resolved
@elsom25
Copy link
Collaborator

elsom25 commented Mar 21, 2024

Should we also pop attributes and values out of folders?

red = value!(id: 11, name: "Red")
zoo = value!(id: 12, name: "Zoo")
other = value!(id: 10, name: "Other")
red = value!(id: 11, name: "Red", friendly_id: "test__red")
Copy link
Member

Choose a reason for hiding this comment

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

Curious why we chose double underscore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume it's to help make it clear where the attribute name ends and the value name begins.

This was the format that I saw used in the slack conversation, so I carried it over here and to the parser

@KComrade53
Copy link
Contributor Author

Branch has been rebased off the latest changes to main and is ready for re-review.

I'm going to fix the sort ordering for categories in the parser and add that to this PR

Copy link
Member

@tylerrowsell tylerrowsell left a comment

Choose a reason for hiding this comment

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

Make sure to validate the taxonomy explorer still works.

make build
make serve I believe are the commands to run locally.

@KComrade53
Copy link
Contributor Author

KComrade53 commented Mar 25, 2024

Looks like data is still good

Screenshot 2024-03-25 at 10 21 02 AM

Copy link
Collaborator

@elsom25 elsom25 left a comment

Choose a reason for hiding this comment

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

One required tune, but looks good once the schema is updated!

db/schema.rb Outdated Show resolved Hide resolved
@KComrade53 KComrade53 merged commit 39abb58 into main Mar 25, 2024
3 checks passed
@KComrade53 KComrade53 deleted the kc/values-yaml branch March 25, 2024 19:16
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.

None yet

3 participants