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

Support content and dc namespaces #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jimmybot
Copy link
Contributor

@jimmybot jimmybot commented Sep 7, 2024

Updated PR that adds new functionality into specific namespace handlers

Support dc:creator and content:encoded elements

Adding support by adding the appropriate namespace handlers

<dc:creator>

The <author> element is spec'ed to be the author's email address. In practice,
this is undesirable so many feeds use the <dc:creator> element which is just
the author's name instead. Technically, an item should not have both an
<author> and a <dc:creator> element. If it does, last value will overwrite and
win.

<content:encoded>

This element is the RSS equivalent to Atom's content. Some feeds provide the
full text of the post. In RSS, there is an optional element called
content:encoded that roughly parallels Atom's content element, except that Atom
does not prescribe HTML encoding of the text. This adds support for the
content:encoded element.

Adding support by adding the appropriate namespace handlers

<dc:creator>

The <author> element is spec'ed to be the author's email address. In practice,
this is undesirable so many feeds use the <dc:creator> element which is just
the author's name instead. Technically, an item should not have both an
<author> and a <dc:creator> element. If it does, last value will overwrite and
win.

<content:encoded>

This element is the RSS equivalent to Atom's content. Some feeds provide the
full text of the post. In RSS, there is an optional element called
content:encoded that roughly parallels Atom's content element, except that Atom
does not prescribe HTML encoding of the text. This adds support for the
content:encoded element.
mix.exs Show resolved Hide resolved
Copy link
Owner

@thiagomajesk thiagomajesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jimmybot! I left a few comments, nothing major ❤️🚀

Comment on lines +41 to +42
# dc:creator element test, agnostic to item list order, either first and last items' authors can match
assert List.first(entries).author in ["Amanda Siberling", "Sarah Perez"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a small typo here, the name should be "Amanda Silberling", but I'd rather we have a fixed assert to test if dc:creator takes precedence when author is also specified.

Would you mind creating a test for the new handlers in test/gluttony/handllers that checks for these rules instead?

mix.exs Show resolved Hide resolved
test/gluttony_test.exs Show resolved Hide resolved
assert Enum.count(entries) == 20
end

test "NJ Monitor (Ghost)", %{xml5: xml} do
Copy link
Owner

@thiagomajesk thiagomajesk Sep 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, the tests in this file are more to guarantee that we have the correct structure and that the number of entries after parsing is correct.

Let's create a test for this handler as well. Although, I'm not sure we should be asserting that the content is longer than the description because we can't really enforce this. I think is better if we just test if the structure is correct. Thoughts!?

Because the raw map will now return both description and content, we should test that when we use raw: false option, the final Gluttony.Entry struct will prioritize using content:encoded for :description when provided.

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