Problem/Motivation

As discussed in #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency, for reasons of entity translation and entity access enforcement, we need to be rendering all base entity fields in Views using Field API formatters and the entity rendering pipleline, rather than using special-purpose Views handlers or generic Views data formatters. Views data formatters that are not entity-aware or that do not use the standard entity rendering pipeline simply don't get it right, and it's a critical issue because we need to respect entity access for all entity base fields.

This meta-issue exists to track the progress of converting all entity base fields to use Field API formatters.

One problem that occurs in this conversion is that the Field API formatters do not all have the same functionality as the Views generic formatters. For instance, the timestamp Entity field formatter, just renders out the date using format_date(). Views itself allows you to configure basically everything:

    $form['date_format'] = array(
      '#type' => 'select',
      '#title' => $this->t('Date format'),
      '#options' => $date_formats + array(
        'custom' => $this->t('Custom'),
        'raw time ago' => $this->t('Time ago'),
        'time ago' => $this->t('Time ago (with "ago" appended)'),
        'raw time hence' => $this->t('Time hence'),
        'time hence' => $this->t('Time hence (with "hence" appended)'),
        'raw time span' => $this->t('Time span (future dates have "-" prepended)'),
        'inverse time span' => $this->t('Time span (past dates have "-" prepended)'),
        'time span' => $this->t('Time span (with "ago/hence" appended)'),
      ),
    )'

    $form['timezone'] = array(
      '#type' => 'select',
      '#title' => $this->t('Timezone'),
      '#description' => $this->t('Timezone to be used for date output.'),
      '#options' => array('' => $this->t('- Default site/user timezone -')) + system_time_zones(FALSE),
      '#default_value' => $this->options['timezone'],
    );

So, we may lose some functionality when we convert. A secondary part of this issue is to avoid that by adding functionality to the Entity Field formatters so that they will match options available currently in the Views formatters that we need to not be using any more.

Proposed resolution

In the discussion on this issue, we decided that the correct resolution was to go ahead and switch to using Entity Field formatting, and live with lost functionality as a first step. This is necessary to enforce entity access on all entity fields, and to support translation properly. Since it involves entity access enforcement, all base fields that are not using entity-aware formatting need to be switched to using entity-aware formatting and this is a critical issue.

As a second step, we want to update the Entity Field formatters so that they support all of the options available in Views formatters. This is not as critical -- we can live with reduced functionality if we have to.

Remaining tasks

Here is a list of the Views formatters that (prior to any of this work) are being used for Entity base fields, what functionality is missing from the corresponding Field formatters, and the plans for replacing them. All of the base fields need to be switched to using entity-aware formatters (these are Critical issues). Replacing the missing functionality is not so Critical.

And there's an issue for making sure entity access is tested:
#2462589: Provide test coverage for access checking of all views fields.
And a follow-up to make sure it's complete:
#2464635: Follow-up: Enable additional tests for base entity field access checking in Views

Outstanding issues (see below for details)

Here's the list of formatters to be replaced:

Plugin, status Where used Missing functionality Issues
boolean: \Drupal\views\Plugin\views\field\Boolean -- Done! Used in: EntityViewsData base class Raw output (0/1), several options for the yes/no values, and the fully "custom" option. - #2395613: Make it possible to configure the output of a boolean field on the formatter level updated the formatter.
- #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency makes the base EntityViewsData use it.
DONE!
date: \Drupal\views\Plugin\views\field\Date -- In progress
Used in EntityViewsData. Note that while NodeViewsData and other data classes have specific formats added as pseudo-fields, they are only for arguments (contextual filters), not field output, so we don't have to worry about them here. Selecting a date format, custom date formats, time zone selection, time ago - #2399211: Support all options from views fields in DateTime formatters updated the formatter.
- #2455125: Update EntityViewsData use of generic timestamp to use Field API formatter makes entities use it. Major
Language fields Done! - language: \Drupal\views\Plugin\views\field\LanguageField, used in EntityViewsData
- node_language: \Drupal\node\Plugin\views\field\Language, used in NodeViewsData
- user_language: \Drupal\user\Plugin\views\field\Language, used in UserViewsData
- taxonomy_term_language: \Drupal\taxonomy\Plugin\views\field\Language, used in TermViewsData
Link to entity, display language using native name #2384863: Translation language base field handler should use views field handler, provide unified options adds the formatting options, updates EntityViewsData to use formatter, and removes all three of these entity-specific plugins.
uri: \Drupal\views\Plugin\views\field\Url - Done! Used in EntityViewsData None. #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency changes EntityViewsData to use field formatting.
numeric: \Drupal\views\Plugin\views\field\Numeric - in progress [Critical part is done] - Used in EntityViewsData both for regular numeric fields and Entity Reference fields.
- Used in FileViewsData for fields on file_usage table: id, count [not base entity fields!]
- Used in CommentViewsData for fields on comment_entity_statistics table: comment_count, last_comment_uid [not base entity fields!]
Plural formatting, such as "1 thing" vs. "2 things" - #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency changed EntityViewsData to use field formatting.
- Add settings for plural: #2449597: Number formatters: Make it possible to configure format_plural on the formatter level, now closed as a duplicate of #2545730: Misuse of formatPlural() in Numeric field prefix/suffix. This depended on #2453761: Views numeric formatter's plural formatting setting incompatible with many languages, which in turn depended on #2454859: Not possible to format plural an already translated string; both of those are done.
- Cannot convert non-base fields to use field formatting, so the rest are not issues.
standard: \Drupal\views\Plugin\views\field\Standard - Done! Used in EntityViewsData both for regular text fields and Entity Reference fields.
Used in FileViewsData for fields on file_usage table: module, type [not base table!]
Used in CommentViewsData for fields comment_field_data.field_name, comment_entity_statistics.field_name [not base table!], comment_entity_statistics.entity_type [not base table!]
None. - #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency changes EntityViewsData to use field formatting.
- #2455131: Field comment_field_data.field_name should be using Field API formatter for CommentViewsData use on comment_field_data.field_name
- Database fields that are not on base tables cannot use field formatting, so the others are not issues.
markup: \Drupal\views\Plugin\views\field\Markup - Done! Used in EntityViewsData, UserViewsData (signature field) None. - #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency changes EntityViewsData to use field formatting.
- #1548204: Remove user signature and move it to contrib is for where it is used in UserViewsData for the Signature field.
Plugins from Node module - Done! These are used in NodeViewsData:
- node (fields: nid, title)
- node_type (fields: type) [entity bundle... not sure what to do with this, but it is probably OK?]
- node_link (pseudo-field: view_node)
- node_link_edit (pseudo-field: edit_node)
- node_link_delete (pseudo-field: delete_node)
- node_path (pseudo-field: path)
- node_bulk_form (pseudo-field: node_bulk_form)
- node_revision (field: title on node_revision)
- node_revision_link (pseudo-field: link_to_revision on node_revision)
- node_revision_link_revert (pseudo-field: revert_revision on node_revision)
- node_revision_link_delete (pseudo-field: delete_revision on node_revision)
Probably not. - #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency removes node handler for base table title field.
- Post-2342045, the remaining plugins in NodeViewsData for fields are:
-- node_type (bundle field, not sure what to do with it, as it's not really an entity field).
-- Various link-related fields/plugins: node_link, node_link_edit, node_link_delete, node_path, node_revision_link, node_revision_link_revert, node_revision_link_delete. See #2322949: Implement generic entity link view field handlers.
-- node_bulk_form -- this appears to be entity-aware so is probably OK
-- node_revision: #2456599: Field node_field_revision.title needs to use an entity-aware formatter in Views Critical [done]
- Post-2322949 and 2456599, the remaining link plugins that are still in NodeFieldData are:
-- node_path
-- node_bulk_form
-- node_revision_link
-- revert_revision
-- delete_revision
-- search_score
These are probably fine to leave as-is. Search is not in the base entity tables. Revision links are not covered by the generic entity link plugins. Node path... probably not necessary but not really horrible to have. And the bulk form is its own thing too. So, this is done!
Plugins from User module - Done! Here's the list:
- user (fields: NodeViewsData - uid, UserFieldData - uid, CommentViewsData - uid)
- user_name (field: UserFieldData - name)
- user_mail (field: UserFieldData - mail)
- user_link_edit (pseudo-field: UserFieldData - edit_node, which despite the name is actually a link to edit the user)
- user_link_cancel (pseudo-field: UserFieldData - cancel_node, which despite the name is actually a link to cancel the user)
- user_data (pseudo-field: UserFieldData - data)
- user_bulk_form (pseudo-field: UserFieldData - user_bulk_form)
- user_roles (UserFieldData - roles_target_id in user__roles table - not on a base table)
- user_permission (UserFieldData - permission pseudo-field in user__roles table - not on a base table)
Probably not. - Post-2342045, the base field plugins remaining in UserViewsData (as well as Node/Comment where they reference User) on base tables are:
-- user_mail: see #2456691: User email field need to use Field-Entity-aware formatters in Views Critical [done]
-- user_name: see #2454145: Replace user_name handler with Field API formatter Major [done]
-- user: see #2470093: Views plugin 'user' needs to be replaced with entity-aware 'field' plugin Major [done]
-- various link fields/plugins: view_user, user_link_edit, user_link_cancel - see #2322949: Implement generic entity link view field handlers but not sure if that will actually fix these. Probably not critical anyway since the worst is someone gets a link they shouldn't and it 403s on them.
-- user_data: This gets data from the "user_data" service. See #2456695: Remove or fix user_data Views integration: make sure it does proper entity access/translation and filtering, or get rid of it -- decided this was OK as-is.
-- user_bulk_form -- this appears to be entity-aware so is probably OK
Plugins from Aggregator module - Done! - aggregator_title_link: in AggregatorFeedViewsData and AggregatorItemViewsData for the title fields
- aggregator_xss: in AggregatorItemViewsData for the author and description fields
The XSS plugin derives from the Views XSS base plugin. We would need to have XSS filtering available on a Field UI formatter. - #2456701: Replace aggregator_title_link Views formatter with Field API formatter for aggregator_title_link - Major [done]
- #2455149: Aggregator xss fields should be using Field/Entity formatters for aggregator_xss formatter - Major [done]
Plugins from Comment module - Done! - comment: Used in CommentViewsData for fields subject, cid
- comment_username: Used in CommentViewsData for field name
- comment_link: Used in CommentViewsData for pseudo-field view_comment
- comment_link_edit: Used in CommentViewsData for pseudo-field edit_comment
- comment_link_delete: Used in CommentViewsData for pseudo-field delete_comment
- comment_link_approve: Used in CommentViewsData for pseudo-field approve_comment
- comment_link_reply: Used in CommentViewsData for pseudo-field replyto_comment
- comment_depth: Used in CommentViewsData for field thread
- comment_last_timestamp: Used in CommentViewsData for field last_comment_timestamp [this is on comment_entity_statistics, not on base entity table]
- comment_ces_last_comment_name: Used in CommentViewsData for field last_comment_name [not base table]
- comment_ces_last_updated: Used in CommentViewsData for field last_updated [not base table]
Probably not. - Post-2342045, the Comment module base field plugins remaining in CommentViewsData on base tables are:
-- 'comment', used on comment_field_data.subject, comment_field_data.cid; 'comment_depth' used for comment_field_data.thread. Issue: #2456705: Comment views field handlers need to be replaced with field/entity aware handlers Critical [done]
-- 'comment_username' used for comment_field_data.name - Issue: #2454163: Replace comment_username handler with generic views handler Critical [done]
-- various link fields/plugins: comment_link, comment_link_edit, comment_link_delete, comment_link_approve, comment_link_reply - see #2322949: Implement generic entity link view field handlers but not sure if that will actually fix these. Probably not critical anyway since the worst is someone gets a link they shouldn't and it 403s on them.
Plugins from Block Content module - done! - block_content: Used in BlockContentViewsData for field id on base table, info on field data table
- block_content_type: Used in BlockContentViewsData for field type on field data table
Probably not. #2456707: Block Content views field handlers need to be replaced with entity-aware handlers
Plugins from File module - Done! - file: In FileViewsData for fields fid, filename
- file_uri: In FileViewsData for field uri
- file_filemime: In FileViewsData for field filemime
- file_extension: In FileViewsData for field extension
- file_size: In FileViewsData for field filsize
- file_status: In FileViewsData for field status
Probably not? #2456709: File views handlers need to be replaced with entity-aware formatters Critical [done]
Plugins from Taxonomy module - Done! Used in TermViewsData:
- term_edit_link - field edit_term
- taxonomy - field name
TBD - Post-2342045, the remaining plugins in TermViewsData for fields are:
-- 'taxonomy', used in taxonomy_term_field_data.name #2456713: Custom taxonomy field views handler needs to be replaced with generic Field API handler Critical [done]
-- Various link-related fields/plugin: term_link_edit. See #2322949: Implement generic entity link view field handlers but not sure if that will actually fix this. Probably not critical anyway since the worst is someone gets a link they shouldn't and it 403s on them
- 'term_name' - which derives from an entity-aware plugin, so it's OK
Other random plugins - Done! - content_translation_link (Used in pseudo-fields called translation_link in: NodeViewsData, UserViewsData, BlockContentViewsData, CommentViewsData, TermViewsData)

- xss (used in fields: NodeViewsData - revision_log, AggregatorFeedViewsData - description, BlockContentViewsData - revision_log)

- search_score (fields: NodeViewsData - node_search_index.score) [not an Entity field, cannot use Field rendering]

entity_label (fields: FileViewsData - file_usage.entity_label) [not an Entity field, cannot use field rendering]

XSS filtering - content_translation_link -- see #2322949: Implement generic entity link view field handlers Probably not critical anyway since the worst is someone gets a link they shouldn't and it 403s on them
- AggregatorFeedViewsData use of xss - #2455149: Aggregator xss fields should be using Field/Entity formatters
- Other uses of xss - #2455153: Switch revision log views fields to use 'field' formatter

User interface changes

Entity fields will be formatted using Entity rendering in Views. Entity fields in and out of Views will have more formatting options available, and they'll be consistent with what is available in Views for base data fields.

API changes

None.

Comments

dawehner’s picture

Gábor Hojtsy’s picture

I don't think it would be a problem to have the options as long as its consistent. I don't think people building the views even necessarily understand why base fields would have reduced features in views. Are you concerned for too many options appearing?

dawehner’s picture

Well, I'm more concerned that the amount of features we have in views aren't visible for base fields, so pretty much all of them.

On the other hand I could imagine that people complain about the amount of options this will add to the field UI.

Gábor Hojtsy’s picture

I think those are the options you get for date fields, then why not provide them for base date fields :)

larowlan’s picture

I think we discuss on a case by case basis. For timestamp I've always envied what views provides relative to field api, so that's a +1 from me.

jhodgdon’s picture

I think maybe some of the comments here are misunderstanding the question in this issue, or else I am?

So... I think the problem is that the proposal on the parent issue is that instead of using the Views field handler classes to format the Base fields of an entity (which is what we are doing now), we would switch to doing what we do for Field UI fields on the entity: we would use Entity/Field Formatters. (And the reason we need to do that is because right now, the base Views field handlers do not understand that the data they're getting is part of an entity, so it's not being access checked or translated, it's just "I got this database field, let's format it".)

However, the Views field handler classes have more formatting options than the Entity/Field formatters. So the question on this issue is basically: is it OK to lose all the nice options? And if not... what can we do about it?

Right?

larowlan’s picture

I see there is a second option, to port the views extra bits to the field api

jhodgdon’s picture

Yes, indeed. I think if the Views formatters for some kind of data have more options than what the basic Field API has, then we should definitely consider porting them to either existing or new Field API Formatters for the Fields that hold that type of data.

I'm not entirely certain this will be possible or appropriate for *all* the settings, but it seems likely that it would be for at least *most* of the settings. If we lose a few possibilities/settings from Views, we may have to live with that; losing a lot is probably not as desirable.

The case in the issue summary here is about time/date. It seems like the stuff shown there would all be good to have on the Field API formatter for dates. It might mean making a new Formatter class as an alternative to whatever ones exist now for DateTime fields, but it seems well within the realm of both Possible to Do and Good to Have.

dawehner’s picture

Priority: Major » Critical

Let's promote it for now, it blocks a critical kinda

plach’s picture

Issue tags: +entity storage
catch’s picture

If I've got the options right, then:

- I'd be fine losing features in Views to resolve the critical (and completely dropping the custom formatters would be great clean-up as a bonus).
- I'd be fine adding those features back to the Field UI formatters separately, but it could use UI review and shouldn't/doesn't have to block release at all.

dawehner’s picture

Even I kinda hate to loose features temporarily I think that is fair. I hope
this UI review won't say that the options are too advanced, because they got added due to the demand of many people.

Can we close this issue now and mark it as "decided"?

dawehner’s picture

Status: Active » Reviewed & tested by the community

So decided seems to be mean RTBC

jhodgdon’s picture

Title: Decide what to do with formatter with less options as views field handlers » [policy] Decide what to do with formatter with less options as views field handlers
Status: Reviewed & tested by the community » Fixed

I think if @catch and @dawehner agree (and no one here seems to disagree), we can call it "decided". This issue is policy so no patch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jhodgdon’s picture

Title: [policy] Decide what to do with formatter with less options as views field handlers » [policy decided, now meta] Decide what to do with formatter having fewer options than views field handlers
Priority: Critical » Normal
Issue summary: View changes
Status: Closed (fixed) » Active

I'm going to reopen this issue as a meta issue, where we can make sure all of the needed fields have been updated according to this policy. Accordingly, downgrading the Priority.

Updating the summary with a survey of the fields we need to address.

jhodgdon’s picture

Issue summary: View changes

Completing the survey... the summary was incomplete as of the last comment.

After completing this table, it strikes me that a unified plugin to make entity links would probably reduce the custom plugins a lot. I'm not convinced currently that all of the link-making plugins specific to Node and other modules are doing the right thing with regard to display language. We'll have to check on that.

Some of the other plugins, like the ones for the node title, can probably be removed in favor of just using field formatting, I think. I believe the main reason these exist now is so they can be made into a link to the entity.

Anyway... at least there is a list now.

dawehner’s picture

Amazing work on the issue summary!!!!

jhodgdon’s picture

Issue summary: View changes

Some updates.

As noted by dawehner on a related issue, fields that are not on the base/data entity tables cannot be rendered using Field rendering. So, noting that in the table in the issue summary.

jhodgdon’s picture

Forgot about this related issue. Added it to the summary table in various places.

Gábor Hojtsy’s picture

I don't think the title is appropriate anymore to the META-ness of this? In many cases, its not about formatters.

jhodgdon’s picture

Title: [policy decided, now meta] Decide what to do with formatter having fewer options than views field handlers » [policy decided, now meta] Make sure Views base fields are using Field API for formatting, and do not lose functionality

How about this?

jhodgdon’s picture

Issue tags: +D8 upgrade path, +D8MI

As Gabor pointed out on related issues, we need to figure out which of these fields are critical to convert now, and make issues for them. The reason is that if we don't do them now, they'll need upgrade paths (affect stored views).

By the way, I'm not sure why this issue has an "entity storage" tag. It's about Views, how is that "entity storage"? Maybe I don't understand the tag though so leaving it for now.

Gábor Hojtsy’s picture

Priority: Normal » Critical

Bumped to critical given there are children which are critical.

xjm’s picture

Title: [policy decided, now meta] Make sure Views base fields are using Field API for formatting, and do not lose functionality » [meta] Make sure Views base fields are using Field API for formatting, and do not lose functionality

Agreed, we can downgrade this once we're sure everything that needs to be critical (and upgrade path blocking) is tagged and filed or complete.

Also retitling because the word "policy" makes me scan past it. :P

Gábor Hojtsy’s picture

Title: [meta] Make sure Views base fields are using Field API for formatting, and do not lose functionality » [META] Make sure Views base fields are using Field API for formatting, and do not lose functionality
Issue summary: View changes

Fixing intro of issue summary.

jhodgdon’s picture

The issue that was marked as this one's parent is now marked as a child. We don't want any time paradoxes of being your own grandparent!

dawehner’s picture

HAHA, yeah I have seen that but just could not motivate me to fix it :)

catch’s picture

Using field API formatters seems critical since it'll be a pain to sort out the upgrade path afterwards.

Not losing any configuration options not so much - we could add those in major issues before or after release no?

dawehner’s picture

Not losing any configuration options not so much - we could add those in major issues before or after release no?

Yeah its not part of the criticality. I would vote to move this issue to major, once #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency and #2385443: Test that base entity fields on views respect field level access is in.

Gábor Hojtsy’s picture

But this is critical because unless we solve it for the rest of the base fields that #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency does not cover, we end up not resolving #2385443: Test that base entity fields on views respect field level access either for all base fields, right?

dawehner’s picture

does not cover, we end up not resolving #2385443: [PP-1] Base entity fields using 'standard' plugin added via EntityViewsData to not respect field level access either for all base fields, right?

We can indeed switch to the formatters already, but I'd hate to not longer have all the options of views field handlers, but I agree with @catch that
those options are not critical.

So for me the way to do would be, fixing the other two critical, then add the settings to the formatters and switch them over. If time is not enough, we skip the
settings part of it before the RC.

Gábor Hojtsy’s picture

Can we resolve #2385443: Test that base entity fields on views respect field level access across all base fields without switching all base fields over?

dawehner’s picture

Can we resolve #2385443: [PP-1] Base entity fields using 'standard' plugin added via EntityViewsData to not respect field level access across all base fields without switching all base fields over?

No we can't.

Gábor Hojtsy’s picture

Ok, so we either need to make that turn over the rest of the base fields without the formatter updates all at once or make the rest of the subissues happen here.

dawehner’s picture

Ok, so we either need to make that turn over the rest of the base fields without the formatter updates all at once or make the rest of the subissues happen here.

Yeah exactly. The first bit is a backup plan, IMHO

dawehner’s picture

jhodgdon’s picture

jhodgdon’s picture

Issue summary: View changes

Updating summary and filing new issues that were missing. New issues filed were listed with this one as Parent. Here's a list if you want to follow them or better yet work on them:
#2455125: Update EntityViewsData use of generic timestamp to use Field API formatter
#2455131: Field comment_field_data.field_name should be using Field API formatter

There are more to be evaluated, but getting this in for now...

jhodgdon’s picture

xjm’s picture

Thanks @jhodgdon for getting those filed!

Now that #2342045: Standard views base fields need to use same rendering as Field UI fields, for formatting, access checking, and translation consistency and #2385443: Test that base entity fields on views respect field level access are in, I looked over the list of remaining work in the summary. As far as I can tell, none of the outstanding work is critical, nor are the feature regressions (though certainly a lot of it is major). It seems like for the most part, the remaining issues are either Views not leveraging the formatter functionality for for specific fields (and therefore some fields are not properly rendered or translated, do not support in-place editing, etc.). But all the access checking issues are resolved, correct?

Specifically:

Among the plugins we haven't checked yet, do we think any of them have potentially critical issues in not using Entity API formatters?

dawehner’s picture

remaining work in the summary. As far as I can tell, none of the outstanding work is critical, nor are the feature regressions (though certainly a lot of it is major). It seems like for the most part, the remaining issues are either Views not leveraging the formatter functionality for for specific fields (and therefore some fields are not properly rendered or translated, do not support in-place editing, etc.). But all the access checking issues are resolved, correct?

Well, we have still tons of handlers ... and every usage blocks translation support to work.

So for example we still have no really support for links to revisions, or even just displaying the username in a proper way. You could all consider this as bugs,
but still, we are bypassing the entity access API for fields not using the formatters.

Gábor Hojtsy’s picture

Looks like the key sentence then is we are bypassing the entity access API for fields not using the formatters. If we considered the prior ones critical, then why not these?

dawehner’s picture

Looks like the key sentence then is we are bypassing the entity access API for fields not using the formatters. If we considered the prior ones critical, then why not these?

Exactly, on top though there is also the argument of translatability.

jhodgdon’s picture

Indeed. Any base field that is not using an entity-aware formatter is bypassing entity access (critical) and has completely broken translatability (probably also critical; at least major; but it doesn't matter because the fix for entity access is the same fix: use the Field API).

The tests/patches so far have fixed some of these critical sub-issues, but not all, and the remaining ones are still critical. The loss of functionality patches are not critical, but we're not down to just those yet.

I have to finish going through this list and filing some more issues. Plan to do that shortly...

xjm’s picture

@dawehner, @Gábor Hojtsy, thanks! It wasn't clear to me that entity access was still being bypassed for these fields with the two patches that already went in. So let's bump each of those outstanding issues to critical where access control is bypassed.

A summary and title update would also be great, because I did read the summary very carefully and didn't understand that these fields were still bypassing access control.

jhodgdon’s picture

Updating summary. I still need to go through the remaining "needs evaluation" items and evaluate them. Plan to get to that today if possible.

jhodgdon’s picture

Issue summary: View changes

I elevated a couple of the child issues to Critical, and filed a new one. And added a few notes to the summary table. Running out for a bit but I got through evaluating a few more and changed status accordingly... a bit more to do later today.

jhodgdon’s picture

Issue summary: View changes

Got through the User plugins. #2456695: Remove or fix user_data Views integration: make sure it does proper entity access/translation and filtering, or get rid of it is pretty scary -- someone please tell me I'm wrong?!? A few more left to evaluate...

jhodgdon’s picture

Issue summary: View changes

Got through Aggregator, Comment, Block Content, File... almost through the eval...

jhodgdon’s picture

Issue summary: View changes

OK, finished filling in all the issues, phew! I guess I single-handedly upped the critical count by about 10 today. However, as xjm said in IRC, they're small, known, doable criticals rather than this big "Who knows what we need to do" critical meta. I think I got them all. One of the new ones already even has a patch. ;)

jibran’s picture

AFAIK all the children of critical meta should be major until we have 2 children. Why are they all critical?

jhodgdon’s picture

@jibran #52 - because xjm asked me to make them all critical. ;)

Dom.’s picture

Hi,
I have a concern with content types displayed in views that seems related. Please see: #2458223: Duplicated field handlers in field UI for some base table fields

jhodgdon’s picture

Issue summary: View changes

Update status of meta.

xjm’s picture

@jibran re: #52:

AFAIK all the children of critical meta should be major until we have 2 children. Why are they all critical?

Any issue that has critical implications on its own should be filed as critical. See the definition of a critical issue. (The one exception would be e.g. the router or CMI conversions where there were hundreds of children that were all blockers to removing the legacy system, and it simply wasn't reasonable to mark them all as critical and still have a usable issue queue due to their small scope individually. That's more of an issue management decision.) But during this phase of the release cycle especially, really every issue which is critical should be filed as such so that the extent of our technical debt is clear.

There is no rule like "Children of issue priority X always have priority Y" or "Children of issue priority X have priority Y when there are N of them". A critical meta can have any number of non-critical children, or vice versa.

Issues where there is unexpected disclosure of field data due to access checking being inconsistently skipped for some Views fields but not others are covered by this point in the criteria for critical issues:

Expose security vulnerabilities.

@jhodgdon went through the children and figured out which issues those were (which is awesome and needed work), per my request in #46.

Hope that helps clarify. :)

webchick’s picture

Issue tags: +Critical Office Hours

Per xjm, the subset of issues in this meta that are critical would make good candidates for Critical Office Hours. Tagging.

jhodgdon’s picture

effulgentsia’s picture

Issue tags: -Critical Office Hours

Per xjm, the subset of issues in this meta that are critical would make good candidates for Critical Office Hours.

I just now tagged the relevant subset, so removing the tag from this meta.

jhodgdon’s picture

Issue summary: View changes

Comment issues have been split up. And updating status.

jhodgdon’s picture

jhodgdon’s picture

Issue summary: View changes

Updated meta status...

jhodgdon’s picture

Issue summary: View changes

I just went through the existing Entity ViewsData classes to see what is left. There was one that got overlooked -- the 'user' plugin, used in UserViewsData, NodeViewsData, CommentViewsData. Added to meta and commented on an issue that was added to the meta that was addressing the user_name plugin (no idea how they're different or why/if we need either one, most likely not...).

jhodgdon’s picture

xjm’s picture

Priority: Critical » Major

I discussed #2470093: Views plugin 'user' needs to be replaced with entity-aware 'field' plugin a bit with both @dawehner and @alexpott. Based on @jhodgdon's review, I think all the access-related bugs from this have been fixed! So I am downgrading this to major. Edit: Other than #2456599: Field node_field_revision.title needs to use an entity-aware formatter in Views, I meant, which is already critical on its own.

The next step is to finish the remaining bugs and fix our multilingual support.

Great work everyone! And thanks especially to @jhodgdon for keeping things on track here.

xjm’s picture

Issue summary: View changes

Adding a short list of the remaining issues to the summary.

xjm’s picture

Issue tags: +D8 Accelerate Dev Days
xjm’s picture

Issue summary: View changes
jhodgdon’s picture

Issue summary: View changes

Wow, lots of progress at DDD! Updated summary. Just a few things left to do really...

mpdonadio’s picture

{#2399211] and #2455125: Update EntityViewsData use of generic timestamp to use Field API formatter are really just waiting on consensus and/or input from the Views maintainers on what we want to do about the timeago formats, then I can wrap them up.

jhodgdon’s picture

Issue summary: View changes

Updating issue summary. Wow, lots of progress! The numeric and time stamp issues are all that are left. Great work everyone!

jhodgdon’s picture

Issue summary: View changes

Updating meta status

mpdonadio’s picture

jhodgdon’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.