Integrate Popular content with Views [#1843264]

Comment	File	Size	Author
#10	fixed_for_default_views_block.patch	9.88 KB	man-1982
#9	ym_popular_content_block.jpg	77.92 KB	Konstantin Komelin
#5	added_support_views_ver1.1.patch	53.54 KB	man-1982
#1	added_support_views__added_css_file.patch	64.44 KB	man-1982
#1	added_support_views__added_css_file.patch	64.44 KB	man-1982

Comment #1

man-1982 commented 18 November 2012 at 16:36

Status	File	Size
new	added_support_views__added_css_file.patch	64.44 KB
new	added_support_views__added_css_file.patch	64.44 KB

Added support for module views. Added default views block. Please, see "admin/structure/views". Done.

Log in or register to post comments

Comment #2

kalabro

she/her

commented 18 November 2012 at 21:24

Not sure that we can modify code in hook_schema in this way. Now I get annoying error before updating:

SQLSTATE[42000]: Syntax error or access violation: 1075 Incorrect table definition; there can be only one auto column and it must be defined as a key

Log in or register to post comments

Comment #3

Konstantin Komelin commented 19 November 2012 at 03:31

Status:

Active

» Needs work

Thanks, kalabro!
Besides this patch includes changes for #1643374: Remove clear: both;.

man-1982, it would be better to recreate patch according to the comments:
1) Synchronize code with latest branch changes
2) Extract changes for #1643374: Remove clear: both; into a separate patch and attach it to the appropriate issue.
3) Fix issues with database #2

----

Спасибо, kalabro!
Кроме того этот патч включает изменения для #1643374: Remove clear: both;.

man-1982, было бы лучше пересоздать патч с учетом комментариев:
1) Синхронизировать код с последними изменениями в бренче
2) Выделить изменения для #1643374: Remove clear: both; в отдельный патч и приложить его к соответствующему тикету.
3) Исправить проблемы с базой согласно #2

Log in or register to post comments

Comment #4

kalabro

she/her

commented 20 November 2012 at 14:32

Time to review :)

1. To fix #2 we can use the 4th parameter from db_add_field() function instead of db_add_primary_key():

/**
 * Added primary key.
 */
function yandex_metrics_reports_update_7001() {
  $spec = array(
    'description' => 'The id for url.',
    'type' => 'serial',
    'unsigned' => TRUE,
    'not null' => TRUE,
  );
  db_add_field('yandex_metrics_reports_popular_content', 'yid', $spec, array('primary key' => array('yid')));
}

2. When try to add field “Yandex.Metrics: Title for url” I get: Broken/missing handler. Handler views/handlers/views_handler_field_ym_title.inc should be listed in module .info file.

; Views handlers
files[] = views/handlers/views_handler_field_ym_title.inc

3. Please, add Views to dependencies or check if views is enabled in _yandex_metrics_reports_is_popular_content_enabled(). Otherwise we get "Fatal error: Call to undefined function views_get_view() in sites/all/modules/indev/yandex_metrics/yandex_metrics_reports/yandex_metrics_reports.module on line 1081" while cron running.

4. Unnecessary modifications at the end of yandex_metrics.info file.

Log in or register to post comments

Comment #5

man-1982 commented 21 November 2012 at 07:24

Status:

Needs work

» Needs review

Status	File	Size
new	added_support_views_ver1.1.patch	53.54 KB

I've fixed bugs. Please, review my code.

Log in or register to post comments

Comment #6

Konstantin Komelin commented 21 November 2012 at 20:27

Status:

Needs review

» Patch (to be ported)

Found some non-critical issues in comments format and codding standards.
But the patch looks good in a whole and will be ported.

Log in or register to post comments

Comment #7

Konstantin Komelin commented 21 November 2012 at 20:51

Status:

Patch (to be ported)

» Needs work

Good job, Andrey!

Team, thank you, it's great example of team collaboration!

I commited Andrey's patch with small corrections: http://drupalcode.org/project/yandex_metrics.git/commit/cda7b04
And my congratulations with the first commit, Andrey!

Could you port your changes to the 6.x-2.x branch?