Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Conversation

@sjhuang26
Copy link

The following changes were made:

  • Fixed the ID system for the Petition
  • Fixed the expected_sig field to work properly

These changes completely break the existing database file, because the Petition primary key is changed. All the database migration files are changed. The solution I came up with is to delete the database file and reapply the migrations. Let me know if you see a better way to do this.

The following changes were not made:

  • Moving the expected number of signatures to a global constant. What happens if the expected number of signatures changes in the future? In that case, editing the constant would break all of the old petitions.
  • Built in validation of max 3 tags per petition. The implementation of this depends on how the petition-submission form is written. I think this can be dealt with later.

Copy link
Contributor

@garoller garoller left a comment

Choose a reason for hiding this comment

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

I think it's okay to delete the database and apply the new migrations, and I don't know of a different/better way to do this. But if anyone else does, please let us know.

Most of this looks really good, but I think we should decide what we want to do with the expected_sig field in the Petition model in this PR.

Thanks!

@sjhuang26
Copy link
Author

I think expected_sig should be kept as is, as opposed to moving it to a global constant. As stated above: What happens if the expected number of signatures changes in the future? In that case, editing the constant would break all of the old petitions.

Perhaps you have something else in mind for expected_sig ... ?

@garoller garoller dismissed their stale review April 5, 2019 16:54

Dimissing the stale review.

@sjhuang26
Copy link
Author

Created environment variable DEFAULT_EXPECTED_SIGS: defaults to 300. Implemented in base.py settings file, along with the SECRET_KEY.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants