Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
AO3-3496 Mark for Later should be immediate ( take 2 ) #2481
Conversation
zz9pzza
added some commits
May 25, 2016
All checks have passed
All checks have passed
All checks have passed
Some checks were not successful
houndci-bot
reviewed
Jun 8, 2016
| @@ -9,7 +9,7 @@ class WorksController < ApplicationController | ||
| before_filter :check_user_status, :except => [ :index, :show, :navigate, :search, :collected ] | ||
| before_filter :load_work, :except => [ :new, :create, :import, :index, :show_multiple, :edit_multiple, :update_multiple, :delete_multiple, :search, :drafts, :collected ] | ||
| # this only works to check ownership of a SINGLE item and only if load_work has happened beforehand | ||
| - before_filter :check_ownership, :except => [ :index, :show, :navigate, :new, :create, :import, :show_multiple, :edit_multiple, :edit_tags, :update_tags, :update_multiple, :delete_multiple, :search, :marktoread, :drafts, :collected ] | ||
| + before_filter :check_ownership, :except => [ :index, :show, :navigate, :new, :create, :import, :show_multiple, :edit_multiple, :edit_tags, :update_tags, :update_multiple, :delete_multiple, :search, :mark_for_later, :mark_as_read, :drafts, :collected ] |
zz9pzza
referenced this pull request
Jun 8, 2016
Closed
AO3-3496 Mark for Later should be immeadiate #2466
sarken
reviewed
Jun 9, 2016
| @@ -96,12 +96,12 @@ def marked_for_later?(work) | ||
| reading && reading.toread? | ||
| end | ||
| - def marktoread_link(work) | ||
| - link_to ts("Mark for Later"), marktoread_work_path(work) | ||
| + def markasread_link(work) |
sarken
Jun 9, 2016
Owner
This should be mark_as_read_link(work) now that we're using the underscores between each word.
sarken
reviewed
Jun 9, 2016
| - def self.mark_to_read_later(work, user) | ||
| - reading_json = [user.id, Time.now, work.id, work.major_version, work.minor_version, true].to_json | ||
| - REDIS_GENERAL.sadd("Reading:new", reading_json) | ||
| + def self.mark_to_read_later(work, user, toread) |
sarken
Jun 9, 2016
Owner
Should we be calling this mark_for_later instead now, or would that involve changing too many things in too many places?
zz9pzza
Jun 9, 2016
•
Edited 1 time
-
zz9pzza
Jun 10, 2016
Contributor
-
Jun 10, 2016
It would be mark as read if anything, but that name is being used to remove something from the to read table. And I think right now I would get a headache
sarken
Jun 13, 2016
Owner
Okay! In that case, this only needs https://github.com/otwcode/otwarchive/pull/2481/files#diff-47cdfed9877703d7f3ff7b154fa65728R99
sarken
reviewed
Jun 9, 2016
| - <% unless current_user.is_author_of?(@work) || current_user.try(:preference).try(:history_enabled) == false || marked_for_later?(@work) %> | ||
| - <li class="mark"><%= marktoread_link(@work) %></li> | ||
| + <% unless current_user.is_author_of?(@work) || current_user.try(:preference).try(:history_enabled) == false %> | ||
| + <li class="mark"> |
sarken
Jun 9, 2016
Owner
I think the indenting is off here -- this looks like one space instead of two. Also, line 57 will need to be mark_as_read_link(@work) to go with the change requested in my comment above
sarken
added
the
Reviewed: Needs Coder Action
label
Jun 9, 2016
All checks have passed
zz9pzza
added
Coder has actioned review
and removed
Reviewed: Needs Coder Action
labels
Jun 10, 2016
sarken
added
Reviewed: Needs Coder Action
and removed
Coder has actioned review
labels
Jun 13, 2016
zz9pzza
added some commits
Jun 18, 2016
All checks have passed
zz9pzza
added
Coder has actioned review
and removed
Reviewed: Needs Coder Action
labels
Jun 18, 2016
|
@sarken I think that is all you wanted ? |
zz9pzza commentedJun 8, 2016
Replaces #2466
https://otwarchive.atlassian.net/browse/AO3-3496
I think this has the renames you wanted.