1

Собираю свой "велосипед" по мотивам этого чудесного ответа. Но в моем случае и при создании и при удалении закладки юзером имею ошибки: -при создании advert_id остается пустым, -при удалении в вместо bookmark.id берется advert.id, что соответственно дает мне ошибку " ActiveRecord::RecordNotFound in AdvertsController#bookmark Couldn't find Bookmark with 'id'=1". где 1 это id объявления.

Где не прав? Заранее благодарен.

user model

has_many :adverts
has_many :bookmarks, dependent: :destroy
has_many :bookmark_adverts, through: :bookmarks, source: :advert

advert model

belongs_to :user
has_many :bookmarks, dependent: :destroy
has_many :bookmarks_user, through: :bookmarks, source: :user

bookmark model

belongs_to :advert
belongs_to :user
validates :advert_id, uniqueness: { scope: :user_id, message: 'has already been bookmarked' }

routes

  resources :adverts, path: 'obyavleniya' do
    # resources :bookmarks, only: [:create, :destroy]
    put :bookmark, on: :member
  end

adverts controller

class AdvertsController < ApplicationController
  load_and_authorize_resource
  before_action :authenticate_user!, except: [:index, :show]
  before_action :load_advert, only: [:edit, :update, :destroy, :bookmark]


  def index
    @adverts = Advert.all
  end

  def show
  end

  def create
    @advert = current_user.adverts.create(advert_params)
  end

  def edit
  end

  def update
  end

  def destroy
    @advert.destroy
  end

  def bookmark
    type = params[:type]
    if type == "bookmark"
      @bookmark = current_user.bookmarks.create(advert_id: params[:advert_id])

      if @bookmark.save
        redirect_to :back, notice: "You bookmark #{@advert.id}"
      else
        redirect_to :back, notice: "NO bookmark ;("
      end

    elsif type == "unbookmark"
      current_user.bookmarks.delete(advert_id: params[:advert_id])
      @bookmark = Bookmark.find(params[:id])

      if @bookmark.destroy
        redirect_to :back, notice: "Unbookmark #{@advert.id}"
      else
        redirect_to :back, notice: "NO Unbookmark ;("
      end

    else
      redirect_to :back, notice: 'Nothing happened.'
    end
  end

  private
    def load_advert
      @advert = Advert.find(params[:id])
    end

    def advert_params
      params.require(:advert).permit(:title, :content, :user_id)
    end
  end

advert show views

- if current_user
  = link_to "bookmark",   bookmark_advert_path(@advert, type: "bookmark"), method: :put
  = link_to "unbookmark", bookmark_advert_path(@advert, type: "unbookmark"), method: :put

1 ответ 1

0

Похоже, что прямо перед тем, как найти закладочку, вы её стёрли. А вот что с созданием, невооружённым глазом не видно, берите binding.pry и смотрите, что у вас там за параметры. Или не берите, потому что на мой взгляд практически весь этот механизм закладок нужно выбросить ¯\_(ツ)_/¯

Заметьте, какой у вас переусложнённый вышел метод bookmark. Сразу два разных действия выполняет, ещё и с другим ресурсом, чем назначен контроллеру, у-ух!

А в комментариях написана очень здравая мысль! Давайте ею воспользуемся и...

...всё нафиг переделаем!

Начнём с самого начала. С точки зрения пользователя закладка — единичный (на одну рекламку) ресурс без дополнительных данных, который может существовать, а может и нет. Отразим это в маршрутах, используя понятие "ресурса" по максимуму:

resources :adverts do
  resource :bookmark, only: [:create, :destroy]
end

В результате для закладок получаем два аккуратных маршрута:

POST    /adverts/:advert_id/bookmark
DELETE  /adverts/:advert_id/bookmark

Обращаю внимание, ресурс единичный, отдельного id в маршрутах для него нет. Ведь зачем он? По advert_id и текущему пользователю (user_id) можно однозначно найти нужную закладку.

Отдельному ресурсу — отдельный контроллер.

В нём два отдельных метода, которые работают, по сути, с одним и тем же скоупом, в котором должна быть одна запись или не быть записей вообще. Так что скоуп выносим в отдельный метод и создание/удаление становится простым просто до неприличия:

# Смотрим,    кто обратился        и за чем
before_action :authenticate_user!, :load_advert

def create
  bookmarks_scope.first_or_create # делаем в скоупе запись
end

def destroy
  bookmarks_scope.destroy_all     # трём из скоупа запись, по идее одну
end

private

def load_advert
  @advert = Advert.find(params[:advert_id])
end

def bookmarks_scope
  current_user.bookmarks                # закладки текущего пользователя
              .merge(@advert.bookmarks) # ...и указанной рекламки
end

renderы и редиректы по вкусу.

По объёму кода сопоставимо с одним вашим методом bookmark, и при этом гораздо проще, а вдобавок идемпотентно: если попытаться совершить действие, которое уже было сделано, ничего не произойдёт. Здесь это имеет смысл, пользователю-то важен конечный результат: чтобы закладка или точно была, или её точно не было, не более. Максимум: в результате race condition'а может всплыть RecordNotUnique, но его можно поймать и игнорировать (т. к. в данном контексте он означает "запись уже есть, ничего делать не надо).

Ещё найденные дыры:

  • has_many :bookmarks_user — юзера определённо много.
  • bookmark_adverts -> bookmarked_adverts. Так больше похоже на английский.
8
  • Спасибо за развернутый ответ.<br/> Но при выводе ссылок получаю ошибку:<br/> Routing Error No route matches [GET] "/obyavleniya/1/bookmark" resources :adverts, path: 'obyavleniya' do resource :bookmark, only: [:create, :destroy] end ссылку вывожу: <br/> = link_to "bookmark", advert_bookmark_path(advert_id: @advert.id)
    – alexin
    19 дек 2015 в 15:08
  • @alexin естественно, метод-то не указали, а ссылкв по умолчанию GET. Строго говоря, ссылки могут только GET, а остальные это уже магия jquery-ujs.
    – user181100
    19 дек 2015 в 15:26
  • понял. спасибо.
    – alexin
    19 дек 2015 в 17:17
  • С добавлением разобрался. ` = link_to "bookmark", advert_bookmark_path(advert_id: @advert.id), method: :post, remote: true` А вот с удалением что то застрял. Как вывести на удаление закладки текущего пользователя для текущего объявления?
    – alexin
    19 дек 2015 в 19:23
  • @alexin "текущего пользователя" в моём варианте не нужно обозначать в URL вообще.
    – user181100
    19 дек 2015 в 19:35

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.