-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DS-97] Instructor Reference field is not displayed on Schedule/Teasers #13
Conversation
Composer patch is required for proper inheritance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some important changes needed
src/Controller/EventsController.php
Outdated
@@ -8,6 +8,7 @@ | |||
use Drupal\Core\Datetime\DrupalDateTime; | |||
use Drupal\Core\Extension\ModuleHandlerInterface; | |||
use Drupal\openy_gated_content\VirtualYAccessTrait; | |||
use Drupal\taxonomy\Entity\Term; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add dependency to taxonomy module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This use
was outdated and was replaced with the entity queries. I've removed it instead.
src/Controller/EventsController.php
Outdated
public function getInheritedFieldLabels(int $id, string $field): array | ||
{ | ||
// Get the field values from the parent series. | ||
$series_values = \Drupal::service('entity_type.manager') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use Dependency Injection and avoid static calls for better container performance and cacheability
src/Controller/EventsController.php
Outdated
->{$field} | ||
->referencedEntities(); | ||
// Get the field values from the instance. | ||
$instance_values = \Drupal::service('entity_type.manager') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use Dependency Injection and avoid static calls for better container performance and cacheability
Related Issue/Ticket:
PLEASE CHECK BASE BRANCH FOR YOUR PR
ONLY urgent and approved fixes for point release should go to master branch
https://yusa.atlassian.net/browse/DS-97
Instructor data from
field_gc_instructor_reference
was not previously displaying on event teasers or the VY schedule page, as per #12. With this change, Instructors from that field will be used in the display and if the field is not populated any data in the deprecatedHost's Name
field will be used.Inheritance should go as follows:
Steps to test:
Host's Name
field you may need to re-enable the field atadmin/structure/events/instance/types/eventinstance_type/virtual_meeting/edit/form-display
admin/structure/events/instance/types/eventinstance_type/live_stream/edit/form-display
admin/structure/events/series/types/eventseries_type/virtual_meeting/edit/form-display
admin/structure/events/series/types/eventseries_type/live_stream/edit/form-display
Quality checks:
Please check these boxes to confirm this PR covers the following cases:
hook_update_N
or other means.team in Slack