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

[GroupBundNaturschutzBridge] Add bridge and adjust XPathAbstract #2445

Merged
merged 3 commits into from
Mar 29, 2022
Merged

[GroupBundNaturschutzBridge] Add bridge and adjust XPathAbstract #2445

merged 3 commits into from
Mar 29, 2022

Conversation

dweipert-3138720606
Copy link
Contributor

This adds a bridge for the subdomains of https://bund-naturschutz.de.
The local groups with corresponding subdomains are listed here: https://www.bund-naturschutz.de/ueber-uns/organisation/kreisgruppen-ortsgruppen

I named the bridge GroupBundNaturschutzBridge because it doesn't actually get the news from bund-naturschutz.de, but from its groups. Where each group has its own subdomain.

All groups are working, except for the ones I commented out, mainly because they didn't use the subdomain pages and therefore had a different HTML layout.

I'm not sure about the exact name, description and parameter name regarding what should be written in english or german.
I guess the parameter name could be Kreisgruppe? Or should it stay Group?

@Niehztog I had to adjust the XPathAbstract.php in the process of creating this bridge.

Some images were named $image.JPG, so I added the case-insensitive modifier to the regex.
Other images had % encoded paths and didn't match the regex correctly, that's why I added the \% there as well.

I'm capturing a DOMText node with the XPath text() function, so I had to adjust the check when getting the value to include DOMText objects.

@Niehztog
Copy link
Contributor

@DRogueRonin Your changes to XPathAbstract.php look meaningful. In my opinion it would make sense to merge them.
Regarding your additions around DOMText: I already have an open pull request adressing this issue: #2324
Maybe we can merge our changes together.

@dweipert-3138720606
Copy link
Contributor Author

You could add my XPathAbstract.php changes to your PR and I could remove them from mine?

Niehztog added a commit to Niehztog/rss-bridge that referenced this pull request Feb 15, 2022
@Niehztog
Copy link
Contributor

I have merged your changes to XPathAbstract.php into my pull request. I am not sure though if and when it will be merged. Lets just wait and see what @em92 says about this.

@dweipert-3138720606
Copy link
Contributor Author

I fixed the code according to the failed phpunit tests, I forgot to check that locally.

@dvikan
Copy link
Contributor

dvikan commented Mar 29, 2022

@DRogueRonin This PR still good?

@Bockiii
Copy link
Contributor

Bockiii commented Mar 29, 2022

bridge

@Bockiii
Copy link
Contributor

Bockiii commented Mar 29, 2022

I'm not the biggest fan of xpathabstract bridges (as its often just "here be prefilled values") but in this case I think it's valuable. Expanding the posts would be even cooler, but I think that would add a lot of complexity (and tbh, I dont think this bridge will have many users).

So thanks, will merge.

@Bockiii Bockiii merged commit c6675dd into RSS-Bridge:master Mar 29, 2022
Kwbmm pushed a commit to Kwbmm/rss-bridge that referenced this pull request Jun 17, 2022
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.

4 participants