Skip to content
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

[RumbleBridge] Facelift, Validation, & Livestreams #4160

Merged
merged 3 commits into from
Jul 29, 2024
Merged

[RumbleBridge] Facelift, Validation, & Livestreams #4160

merged 3 commits into from
Jul 29, 2024

Conversation

NotsoanoNimus
Copy link
Contributor

Cleaned up some of the components of the Rumble Bridge.

  • Added a distinction between All, Videos, & Live tabs on target channel pages. The 'user' option will always just show the All page in the created feed.
  • Added validation of the input parameters. Not that this was a serious risk, but it's nice to be explicit.
  • Fixed the NAME property displaying properly based on whether a specific account is being viewed or just the bridge card is showing.
  • More accessibility (if that's even really necessary).
  • Various punctuation fixes.

This change addresses Issue #4155.

Bridge card now:
image

Feed results look the same as they did before:
image

Copy link

github-actions bot commented Jul 25, 2024

Pull request artifacts

Bridge Context Status
Rumble 1 untitled (current) ✔️
Rumble 1 untitled (pr) ✔️

last change: Monday 2024-07-29 15:26:02

@dvikan
Copy link
Contributor

dvikan commented Jul 28, 2024

[2024-07-28 22:26:08] rssbridge.DEBUG lib/BridgeCard.php(195): The "required" attribute is not supported for lists.

@dvikan
Copy link
Contributor

dvikan commented Jul 28, 2024

linter being a bit annoying. sorry about that

@NotsoanoNimus
Copy link
Contributor Author

@dvikan Oops! I removed the required label from the list type. Thank you for pointing that out.

@dvikan
Copy link
Contributor

dvikan commented Jul 29, 2024

i was unable to add a commit to your branch:

commit 284158169bd7fd1a5231124e40c6f0b324f1949d (HEAD -> NotsoanoNimus/rumble-live-streams)
Author: Dag <[email protected]>
Date:   Mon Jul 29 17:05:58 2024 +0200

    lint

diff --git a/bridges/RumbleBridge.php b/bridges/RumbleBridge.php
index 636e364f..a8841e00 100644
--- a/bridges/RumbleBridge.php
+++ b/bridges/RumbleBridge.php
@@ -77,8 +77,9 @@ class RumbleBridge extends BridgeAbstract
 
     public function getName()
     {
-        return $this->getInput('account')
-            ? ('Rumble.com - ' . $this->getInput('account'))
-            : self::NAME;
+        if ($this->getInput('account')) {
+            return 'Rumble.com - ' . $this->getInput('account');
+        }
+        return self::NAME;
     }
 }

@NotsoanoNimus
Copy link
Contributor Author

NotsoanoNimus commented Jul 29, 2024

i was unable to add a commit to your branch:
. . .

I applied it. I think in my rush to publish changes I may be missing some setup with the toolchain here for debugging and checking the linter locally before pushing. I'll look into that later this evening.

@dvikan dvikan merged commit 6d81d6d into RSS-Bridge:master Jul 29, 2024
7 checks passed
@NotsoanoNimus NotsoanoNimus deleted the NotsoanoNimus/rumble-live-streams branch July 29, 2024 15:56
@dvikan
Copy link
Contributor

dvikan commented Jul 31, 2024

unintended consequence:

https://rss-bridge.org/bridge01/?action=display&bridge=RumbleBridge&account=BannonsWarRoom%2Fvideos&type=channel&format=Atom

rssbridge.ERROR Exception in DisplayAction(RumbleBridge) {
    "type": "Exception",
    "code": 0,
    "message": "Invalid target account.",
    "file": "bridges/RumbleBridge.php",
    "line": 40,
    "url": "https://rss-bridge.org/bridge01/?action=display&bridge=RumbleBridge&account=BannonsWarRoom%2Fvideos&type=channel&format=Atom",
    "trace": [
        "index.php(72): RssBridge->main()",
        "lib/RssBridge.php(103): DisplayAction->execute()",
        "actions/DisplayAction.php(65): DisplayAction->createResponse()",
        "actions/DisplayAction.php(115): RumbleBridge->collectData()",
        "bridges/RumbleBridge.php(40)"
    ]
}

@NotsoanoNimus
Copy link
Contributor Author

If you want to explicitly view a channel's videos, you should not append a /videos onto an account name. That seems like it was a consequence of the way the bridge was initially built, allowing any characters into the Account field. Illegal URL-changing characters should not be permitted in that field.

You should instead use the channel-videos target account type to target the videos page instead: e.g., https://rss-bridge.org/bridge01/?action=display&bridge=RumbleBridge&account=BannonsWarRoom&type=channel-videos&format=Atom

@dvikan
Copy link
Contributor

dvikan commented Jul 31, 2024

this is not me. im only posting logs from rss-bridge.org which I control

yes i agree with your text

@NotsoanoNimus
Copy link
Contributor Author

this is not me. im only posting logs from rss-bridge.org which I control

yes i agree with your text

Makes sense! Apologies for the confusion, perhaps it's a regional English thing, but the 'you' here wasn't calling you out individually. It is more of a general instruction to readers who come across the writing.

NotsoanoNimus added a commit to Solstice-Software/better-rss-bridge that referenced this pull request Aug 8, 2024
* [RumbleBridge] Facelift+media types (livestreams)

* [RumbleBridge] Remove 'required' from list input.

* [RumbleBridge] lint
@dvikan
Copy link
Contributor

dvikan commented Aug 23, 2024

I detected today that this PR introduced a bug. Feed items now contain an additional forward slash in the path.

Example:

<entry>
    <title type="html">England Is Burning</title>
    <published>2024-08-12T03:30:00+00:00</published>
    <updated>2024-08-12T03:30:00+00:00</updated>
    <id>https://rumble.com//v5ae7xp-england-is-burning.html</id>
    <link rel="alternate" type="text/html" href="https://rumble.com//v5ae7xp-england-is-burning.html"/>
    <author>
      <name>[email protected]</name>
    </author>
    <co</content>
  </entry>

The Atom format defaults to using the url as id. This might cause duplicates in feed readers. But they probably trim redundant slages, im not sure.

@NotsoanoNimus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants