Reputation: 307
What I intend is to get the news $new
in a certain language if($new_lang->lang_id == $lang_id)
, then I look in the table of images @foreach($data['images'] as $image)
which corresponds to the news and which is the main one @if($image->imageable_id == $new->id && $image->main == '1')
.
The problem is that when nesting foreachs, the load is very slow, since they are more than 7,000 news and some 70,000 images.
How to get the same result without nesting one foreach inside another?
@foreach($new->langs as $new_lang)
@if($new_lang->lang_id == $lang_id)
@foreach($data['images'] as $image)
@if($image->imageable_id == $new->id && $image->main == '1')
@php
$mini = substr($image->path, 0, strrpos( $image->path, "/"));
$name = substr($image->path, strrpos($image->path, '/') + 1);
$image_mini = $mini.'/mini-'.$name;
@endphp
<div class="crop">
<a href="{{ route('noticia', [$new->id, $new_lang->slug]) }}">{{ HTML::image(file_exists($image_mini)? $image_mini : $image->path, '', array('class' => 'img-responsive ancho_100')) }}</a>
</div>
@endif
@endforeach
@endif
@endforeach
EDITED: added controller and trait
This is the controller:
use ListNoticias;
public function actualidad(Request $request)
{
$data['section_id'] = explode(',', '1,2,3');
$data['ruta'] = 'actualidad';
$data['title'] = __('header.actualidad');
$data['num'] = $request->num;
$url = $request->url();
$data = $this->listNoticias($data, $url);
return view('web.actualidad.listado', compact('data'));
}
And the trait:
trait ListNoticias
{
public function listNoticias($data, $url)
{
$now = date('Y-m-d');
$time = date('H:i:s');
(isset($data['num']))? $num = $data['num'] : $num = '15';
$data['images'] = Image::where('imageable_type', 'App\Models\Noticia')->get();
$data['sections'] = Section::all();
$data['noticias'] = Noticia::with('langs')->where('date', '<', $now)
->where('active', '1')
->whereIn('section_id', $data['section_id'])
->orWhere('date', '=', $now)
->where('time', '<=', $time)
->where('active', '1')
->whereIn('section_id', $data['section_id'])
->orderBy('date', 'desc')
->orderBy('time', 'desc')
->get();
$data['noticias-es'] = [];
$data['noticias-en'] = [];
$data['noticias-pt'] = [];
foreach($data['noticias'] as $row){
foreach($row->langs as $row_lang) {
if ($row_lang->lang_id == '1') {
$data['noticias-es'][] = $row;
} elseif ($row_lang->lang_id == '2') {
$data['noticias-en'][] = $row;
} elseif ($row_lang->lang_id == '3') {
$data['noticias-pt'][] = $row;
} else null;
}
}
// Manual paginate
/* Get current page form url e.x. &page=1
Create a new Laravel collection from the array data
Slice the collection to get the items to display in current page
Create our paginator and pass it to the view
set url path for generated links
*/
$currentPage = LengthAwarePaginator::resolveCurrentPage();
// ES
$itemCollection = collect($data['noticias-es']);
$currentPageItems = $itemCollection->slice(($currentPage * $num) - $num, $num)->all();
$data['noticias-es'] = new LengthAwarePaginator($currentPageItems , count($itemCollection), $num);
$data['noticias-es']->setPath($url);
// EN
$itemCollection = collect($data['noticias-en']);
$currentPageItems = $itemCollection->slice(($currentPage * $num) - $num, $num)->all();
$data['noticias-en'] = new LengthAwarePaginator($currentPageItems , count($itemCollection), $num);
$data['noticias-en']->setPath($url);
// PT
$itemCollection = collect($data['noticias-pt']);
$currentPageItems = $itemCollection->slice(($currentPage * $num) - $num, $num)->all();
$data['noticias-pt'] = new LengthAwarePaginator($currentPageItems , count($itemCollection), $num);
$data['noticias-pt']->setPath($url);
return $data;
}
}
EDITED 2
The first view is a partial and is included in this view that have another two foreach nestead. This is because I need to display records in threes. I have commented on other includes that are not necessary for the problem:
@foreach ($data['noticias-'.$lang]->chunk(3) as $chunk)
<div class="col-xs-12 col-sm-12 col-md-12 col-lg-12 pad_inf_2">
@foreach($chunk as $key => $new)
<div class="col-xs-12 col-sm-12 col-md-4 col-lg-4 pad_der_1">
<article>
//@include('web.index.partials.noticia_etiqueta')
@include('web.actualidad.partials.noticia_image_listado')
//@include('web.index.partials.noticia_date_section')
//@include('web.index.partials.noticia_title')
</article>
</div>
@endforeach
</div>
@endforeach
Model Noticia:
class Noticia extends Model
{
protected $fillable = ['section_id', 'active', 'date', 'time', 'author'];
public function images()
{
return $this->morphMany('App\Models\Image', 'imageable');
}
public function tags()
{
return $this->morphToMany('App\Models\Tag', 'taggable');
}
public function section()
{
return $this->belongsTo('App\Models\Section');
}
public function langs()
{
return $this->hasMany('App\Models\LangNoticia');
}
}
LangNoticia Model:
class LangNoticia extends Model
{
protected $fillable = ['noticia_id', 'lang_id', 'title', 'lead', 'text', 'author_place', 'slug'];
public function noticia()
{
return $this->belongsTo('App\Models\Noticia');
}
}
EDITED 3: Reduced code in the trait with the advices of abr
Trait reduced:
trait ListNoticias
{
public function listNoticias($data, $url)
{
$lang = Session::get('lang');
if($lang == 'en') $lang_id = '2';
elseif($lang == 'pt') $lang_id = '3';
else $lang_id = '1';
$now = date('Y-m-d');
$time = date('H:i:s');
(isset($data['num']))? $num = $data['num'] : $num = '15';
$data['sections'] = Section::all();
$data['images'] = Image::where('imageable_type', 'App\Models\Noticia')->where('main', true)->get();
$noticias = Noticia::with('langs')
->where('date', '<', $now)
->where('active', '1')
->whereIn('section_id', $data['section_id'])
->orWhere('date', '=', $now)
->where('time', '<=', $time)
->where('active', '1')
->whereIn('section_id', $data['section_id'])
->orderBy('date', 'desc')
->orderBy('time', 'desc')
->get();
$data['noticias-'.$lang] = [];
foreach($noticias as $row){
foreach($row->langs as $row_lang) {
if ($row_lang->lang_id == $lang_id) {
$data['noticias-'.$lang][] = $row;
}
}
}
// Paginate
/* Get current page form url e.x. &page=1
Create a new Laravel collection from the array data
Slice the collection to get the items to display in current page
Create our paginator and pass it to the view
set url path for generated links
*/
$currentPage = LengthAwarePaginator::resolveCurrentPage();
$itemCollection = collect($data['noticias-'.$lang]);
$currentPageItems = $itemCollection->slice(($currentPage * $num) - $num, $num)->all();
$data['noticias-'.$lang] = new LengthAwarePaginator($currentPageItems , count($itemCollection), $num);
$data['noticias-'.$lang]->setPath($url);
return $data;
}
}
Upvotes: 0
Views: 118
Reputation: 7194
I think you can access the news main image like this:
$new_lang->noticia->images()->where('main', true)->first();
You could define an accessor in your LangNoticia
model to facilitate the call.
public function getImageAttribute()
{
return $this->noticia->images()->where('main', true)->first();
}
In your view you can remove the foreach()
and call the accessor instead.
@foreach($new->langs as $new_lang)
@if($new_lang->lang_id == $lang_id)
@php
$mini = substr($new_lang->image->path, 0, strrpos( $new_lang->image->path, "/"));
$name = substr($new_lang->image->path, strrpos($new_lang->image->path, '/') + 1);
$image_mini = $mini.'/mini-'.$name;
@endphp
<div class="crop">
<a href="{{ route('noticia', [$new->id, $new_lang->slug]) }}">{{ HTML::image(file_exists($image_mini)? $image_mini : $new_lang->image->path, '', array('class' => 'img-responsive ancho_100')) }}</a>
</div>
@endif
@endforeach
Now you can remove the call to the Image
model in your trait.
Use eager loading when querying the Noticia
model to reduce the number of queries.
Noticia::with(['images' => function($query) {
$query->where('main', true);
}])->get();
Upvotes: 1
Reputation: 2129
Just sharing some thoughts on it. Starting with retrieving the content of Noticias
:
Do you really need all these 3 languages? Are there going to be more in the future? Is it only a segment of these 'Noticias' that you want to show?
Answering these questions you should come up with:
A way to, in the eventuality of adding another language, you won't have to alter the function;
A way to increase the query performance, because fetching 7k records is different from recording the 7k x 154 languages
Do you really need all 7k?
You can, for example, split that query up into several (if you really require all languages) or validate it with an advanced function within your Eager Loading statement.
Noticia::with(['langs' => function ($query) {
//where this lang_id == 1, select it as whatever name you require
}])
.
.
.
Second is, are you sure you want those wheres exactly how they are? You're not filtering much with orWhere or Where. If you plan on narrowing down the search, I advice having a function on your where statement, just like the example shown above. If you'd like to see what the query is all about, write:
dd($data['noticias']->toSql());
Getting the images for all those news is going to be heavy, if you claim to have 70k over 7k news. You don't need to really load everything out of the start, you can use a simple ->paginate(15);
to narrow it down, along side a ->skip(15);
Upvotes: 1