symphonists / pagesfield

Select box field that is populated with all Symphony pages
http://getsymphony.com/discuss/thread/267/
Other
7 stars 9 forks source link

Field still uses direct DB queries #14

Open michael-e opened 11 years ago

michael-e commented 11 years ago

The Migration Guide for 2.3 for Extension Developers points out:

A new manager, PageManager provides an interface for the tbl_pages and tbl_page_types, it is recommended you use the manager instead of querying these tables (as they may not exist in the future)

The Pages Field still uses some queries to tbl_pages_types and one query to tbl_pages. I haven't looked at the new manager yet, so I don't really know if there is a chance to show "best practice" (a.k.a. "follow the recommendations") here. But if it is possible to use the PageManager, it should probably be done.

@designermonkey: What do you think?

designermonkey commented 11 years ago

I did spot these queries. There are a lot of instances of use of the PageManager too, so I think these queries are odd ones out, but I'm happy to take a look soon to see if it can be cleaned up some more.

nitriques commented 10 years ago

I do not know if this issue was fixed and not closed, but the only thing that is not using the PageManager is this one function fetchPageByTypes: https://github.com/symphonists/pagesfield/blob/master/fields/field.pages.php#L125-L187

PageManager::fetchPageByType only accepts one type and we need more. There are other filtering requirement that the PageManager does not cover.

Maybe we should consider moving fetchPageByTypes to the PageManager, but this is a separated issue.

I'll leave the issue open but I think it can be closed.