Conversation
| def archive | ||
| id = params[:lsg_body_id] | ||
| @lsg_body = LsgBody.find(id) | ||
| @lsg_body.archive! | ||
| redirect_to lsg_bodies_path | ||
| end | ||
|
|
||
| def unarchive | ||
| id = params[:lsg_body_id] | ||
| @lsg_body = LsgBody.find(id) | ||
| @lsg_body.unarchive! |
There was a problem hiding this comment.
These endpoints should have an authorized check.
Use pundit for writing policies for auth check
https://github.com/varvet/pundit
There was a problem hiding this comment.
Have rewritten the policy in lsg_bodies_policy. Needs review.
app/models/lsg_body.rb
Outdated
| def archive! | ||
| update(archived: true) | ||
| end | ||
|
|
||
| def unarchive! | ||
| update(archived: false) | ||
| end |
There was a problem hiding this comment.
This can be in the controller itself as we aren't using this method anywhere else. It's good to add a scope instead that will list live LSG's
There was a problem hiding this comment.
Yes. Moved to the controller. Have added a scope to list live lsgs.
db/schema.rb
Outdated
| t.datetime "updated_at", precision: 6, null: false | ||
| t.string "code" | ||
| t.uuid "district_id", null: false | ||
| t.boolean "archive", default: false |
There was a problem hiding this comment.
This looks like a typo. Rollback the current migration and drop archive field from lsg_body
| @@ -0,0 +1,5 @@ | |||
| class AddArchivedToLsgBodies < ActiveRecord::Migration[6.1] | |||
| def change | |||
| add_column :lsg_bodies, :archived, :boolean, default: false | |||
There was a problem hiding this comment.
Should we also add a filed archived_at so that we could figure out when the record was archived?
There was a problem hiding this comment.
Should we keep record of all archive actions when was archived and when was unarchived like we have logs?
There was a problem hiding this comment.
We'll then have :archived, :archived_at fields. Won't it be ideal to keep only one of these? Both the fields can serve as a flag. So should we only go with the :archived_at field?
bijoysijo
force-pushed
the
soft-delete-for-lsgs
branch
from
f260952 to
9de96c5
Compare
bijoysijo
force-pushed
the
soft-delete-for-lsgs
branch
from
9de96c5 to
ba26323
Compare
3 participants
Proposed Changes
closes #100