Skip to content

Conversation

roshan-mathewtech
Copy link
Contributor

146

Comment on lines 2 to 4
attr_accessible :eu_accession_year, :eu_exit_year
belongs_to :geo_entity
validates :geo_entity, :presence => true
Copy link
Contributor

Choose a reason for hiding this comment

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

can we fix the indentation here, please?

Comment on lines 2 to 4
attr_accessible :eu_accession_year, :eu_exit_year
belongs_to :geo_entity
validates :geo_entity, :presence => true
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 also validate that the geoentity needs to be a country(geo_entity_type)

@@ -0,0 +1,10 @@
class CreateEuCountryStatuses < ActiveRecord::Migration
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not totally convinced by the name of this table, I would have gone with something like eu_country_dates maybe? let's have a chat about it

@roshan-mathewtech
Copy link
Contributor Author

Changes are done. Thanks


private

def is_country
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to be used anywhere here, we might need to add a validate line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops!! forgot to add it. Added it now
Thanks

@lucacug
Copy link
Contributor

lucacug commented Oct 14, 2022

looks good now! Tagging as don't merge to use it as the feature branch of all this task

@lucacug lucacug mentioned this pull request Oct 27, 2022
@lucacug
Copy link
Contributor

lucacug commented Nov 10, 2022

closing because superseded by this one #904

@lucacug lucacug closed this Nov 10, 2022
@pdl pdl deleted the feature/cites-trade-eu-entry-exit-date branch August 21, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants