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
    Commented 19 дек. 2015 в 15:08
  • @alexin естественно, метод-то не указали, а ссылкв по умолчанию GET. Строго говоря, ссылки могут только GET, а остальные это уже магия jquery-ujs.
    – user181100
    Commented 19 дек. 2015 в 15:26
  • понял. спасибо.
    – alexin
    Commented 19 дек. 2015 в 17:17
  • С добавлением разобрался. ` = link_to "bookmark", advert_bookmark_path(advert_id: @advert.id), method: :post, remote: true` А вот с удалением что то застрял. Как вывести на удаление закладки текущего пользователя для текущего объявления?
    – alexin
    Commented 19 дек. 2015 в 19:23
  • @alexin "текущего пользователя" в моём варианте не нужно обозначать в URL вообще.
    – user181100
    Commented 19 дек. 2015 в 19:35

Ваш ответ

Нажимая «Отправить ответ», вы соглашаетесь с условиями пользования и подтверждаете, что прочитали политику конфиденциальности.

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