Bit of an odd one but please bear with me:
I have a number of models: Album and Photo.
I wished to be able to update tags on photos from Flickr by clicking a button in my UI.
First I started by creating a rake task that loops through all Photos, grabbed the tags from Flickr and updated the Photo.
I then moved the logic to update a single Photo into a method on the Photo model itself, I called it update_tags_from_flickr
.
I then created another new class method on Photo called update_tags_from_flickr
but this one does the same as my rake task and loops through ALL the photos. That method calls the instance method for each Photo. All Good.
Right, I then added a button next to the name of each Album in my admin area that calls a controller action and passes in the Album id. Like so:
def update_album_tags
@album = Album.friendly.find(params[:album_id])
@album.update_tags_from_flickr
end
I added a new instance method to Album that iterates its photos and calls update_tags_from_flickr
on each one. All good here.
I then thought it might be nice if each “photo tags” update was done as a background job. I created a UpdateTagsFromFlickrJob
and now here is where my confusion started:
The job accepts a Photo. but I had to modify my controller action to add a loop through the Album photos and call the background job on each. Like so:
def update_album_tags
@album = Album.friendly.find(params[:album_id])
@album.photos.each do |photo|
UpdateTagsFromFlickrJob.perform_later(photo)
end
end
I notice that I have that Album.photos
loop in two places. Here in the controller and once (from earlier) in my Album model.
Is this correct?
I know I could update the one in the Album model to use the background job, but not sure if I should? Maybe I want a version that doesn’t use a job? I’m unsure.
Is there a way to reduce the duplication of that loop?
What would you suggest?
Thanks, Neil