Problem/Motivation

A lot of 'get' functions are set in the CloudflareVideoItem.php & CloudflareVideoFormatter.php files.
Let's refactor this & create a general service.

Proposed resolution

Create a Cloudflare Stream service

CommentFileSizeAuthor
#7 interdiff-6_7.txt691 bytestim-diels
#7 3239401-create_a_cloudflare_stream_service-7.patch15.15 KBtim-diels
#6 3239401-create_a_cloudflare_stream_service-6.patch15.11 KBpjbaert
#4 3239401-create_a_cloudflare_stream_service-4.patch13.36 KBpjbaert

Comments

pjbaert created an issue. See original summary.

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented

Sounds good. Let's use that service throughout the whole module and clean up the needed files:

  • CloudflareVideoFormatter
  • CloudflareVideoItem
pjbaert’s picture

pjbaert
Primary language Dutch
commented
Issue summary: View changes

Yes, indeed.
Updated the issue summary as suggested by @tim-diels

pjbaert’s picture

pjbaert
Primary language Dutch
commented
Title: Create a Cloudflare Service » Create a Cloudflare Stream Service
Assigned: pjbaert » Unassigned
Status: Active » Needs review
StatusFileSize
new 3239401-create_a_cloudflare_stream_service-4.patch13.36 KB

This patch creates a Cloudflare Stream service file & interface.
The existing cloudflare_stream.file_extensions is integrated in this new general service.
We now correctly use these new service in our CloudflareVideoItem.php & CloudflareVideoFormatter.php files

Please review the patch.

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
Status: Needs review » Needs work

Didn't had the time to fully test this. but here are some remarks already.

  1. +++ b/src/Plugin/Field/FieldFormatter/CloudflareVideoFormatter.php
@@ -80,14 +80,14 @@ class CloudflareVideoFormatter extends FormatterBase {
-    $this->config = $configFactory->get('cloudflare_stream.settings');

    You should also remove the property definition. 

  2. +++ b/src/Plugin/Field/FieldFormatter/CloudflareVideoFormatter.php
@@ -80,14 +80,14 @@ class CloudflareVideoFormatter extends FormatterBase {
+    $this->cloudflareStream = $cloudflareStream;

    I'm missing the property definition here? 

  3. +++ b/src/Plugin/Field/FieldType/CloudflareVideoItem.php
@@ -89,41 +81,41 @@ class CloudflareVideoItem extends FileItem {
+    $this->cloudflareStream = $cloudflareStream;

    I'm missing the property definition here?

pjbaert’s picture

pjbaert
Primary language Dutch
commented
Status: Needs work » Needs review
StatusFileSize
new 3239401-create_a_cloudflare_stream_service-6.patch15.11 KB

Thanks for the initial feedback. I reworked your suggestions & removed some other nitpicks.
Please review

tim-diels’s picture

tim-diels
He/Him
Primary language Dutch
Location Belgium 🇧🇪
commented
StatusFileSize
new 3239401-create_a_cloudflare_stream_service-7.patch15.15 KB
new interdiff-6_7.txt691 bytes

You forgot the argument for the service. So added this and small nitpick on the service doc.

pjbaert’s picture

pjbaert
Primary language Dutch
commented
Status: Needs review » Fixed

Thanks for your review & feedback.
I consider this done for now. This is pushed to the 8.x-2.x branch

  • pjbaert committed abf2c58 on 8.x-2.x 
    Issue #3239401 by pjbaert, tim-diels: Create a Cloudflare Stream Service

Status: Fixed » Closed (fixed)

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