-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
comment status change migration #2974
Conversation
@jywarren could you please help me here. Thanks! |
I'll try to take a look later today - in the meantime, maybe @publiclab/reviewers could offer some assistance? |
|
||
create_table "answer_selections", force: true do |t| | ||
create_table "answer_selections", force: :cascade do |t| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe @Souravirus has thoughts on this and other edits to the schema example file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this change looks good to me
def up | ||
Comment.all.each do |comment| | ||
comment.status = 1 | ||
comment.save({}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the {}
required? Were you able to test this out locally?
In general it looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, migration worked locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gauravano i'm curious, what's the purpose of this comment.status = 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @siaw23, actually to make spam moderation system for comments, comments would come with 2 status values status = 1
for valid and status = 0
for spam.
Error is: Ah, i think the changes to the schema file are overly broad -- and does this migration affect the schema? I think it may not except for the timestamp, right? |
yes, it shouldn't affect schema, but on running |
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
fixes #0000
-style reference to original issue #@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.
Thanks!