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

item.id is ignored in rss output #96

Closed
ArtskydJ opened this issue Jul 17, 2019 · 4 comments · Fixed by #105
Closed

item.id is ignored in rss output #96

ArtskydJ opened this issue Jul 17, 2019 · 4 comments · Fixed by #105

Comments

@ArtskydJ
Copy link
Contributor

Describe the bug
src/rss2.ts looks at entry.guid, not entry.id as documented (implied) in the readme.
https://github.com/jpmonette/feed/blob/master/src/rss2.ts#L117-L121

Expected behavior

  • entry.guid should be documented OR
  • the code should look at entry.id instead

Actual behavior
entry.guid is not documented, but is being used.

Versions (please complete the following information):

  • feed: I think since version 1.

Additional context
Workaround: submit both the entry.guid and the entry.id

Also, the test didn't catch this issue. If this test line was changed then the tests would catch the issue.
https://github.com/jpmonette/feed/blob/master/src/__tests__/setup.ts#L34

  sampleFeed.addItem({
    title: "Hello World",
-   id: "https://example.com/hello-world",
+   id: "0123456789abcdef-0123-4567-89abcdef"
    link: "https://example.com/hello-world",
    description: "This is an article about Hello World.",
@jpmonette
Copy link
Owner

@ArtskydJ A PR is welcomed to support this 💪🏼.

@ArtskydJ
Copy link
Contributor Author

ArtskydJ commented Aug 4, 2019

Would you rather have me change the docs to guid, or the code to id?

@KnicKnic
Copy link

So I just wasted a bunch of time on this! My personal preference is to remove guid, and bring the rss2 into alignment with atom & json. However this will break backwards compat.

I think the below code would be the best compromise.

    if (entry.guid) {
      item.guid = { _text: entry.guid };
    } else if (entry.id) {
      item.guid = { _text: entry.id };
    } else if (entry.link) {
      item.guid = { _text: entry.link };
    }

@melroy89
Copy link

yea pretty strange bug... Please solve :\

ArtskydJ added a commit to ArtskydJ/feed that referenced this issue Oct 17, 2019
This improves the tests. It also replicates jpmonette#96.
ArtskydJ added a commit to ArtskydJ/feed that referenced this issue Oct 17, 2019
Fixes jpmonette#96 with the code that KnicKnic suggested
jpmonette pushed a commit that referenced this issue Oct 20, 2019
* Change id to a guid

This improves the tests. It also replicates #96.

* Fix RSS ignoring item.id

Fixes #96 with the code that KnicKnic suggested
decebal pushed a commit to decebal/feed that referenced this issue Jun 5, 2020
* Change id to a guid

This improves the tests. It also replicates jpmonette#96.

* Fix RSS ignoring item.id

Fixes jpmonette#96 with the code that KnicKnic suggested
jpmonette added a commit that referenced this issue Jun 6, 2020
* Sanitizing XML feeds

* Adding missing things from previous commit and also support for image with more data

* Fix RSS ignoring item.id (#105)

* Change id to a guid

This improves the tests. It also replicates #96.

* Fix RSS ignoring item.id

Fixes #96 with the code that KnicKnic suggested

* Bump lodash from 4.17.11 to 4.17.15 (#106)

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.11 to 4.17.15.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.11...4.17.15)

Signed-off-by: dependabot[bot] <support@github.com>

* Create FUNDING.yml

* Bump handlebars from 4.1.2 to 4.5.3 (#110)

Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3.
- [Release notes](https://github.com/wycats/handlebars.js/releases)
- [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md)
- [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3)

Signed-off-by: dependabot[bot] <support@github.com>

* Item category for all feed types (#109)

* Item category for all feed types.

* updated version

* use rimraf instead of rm -rf for dev purposes.

* Bump to 4.1.0

* Bump acorn from 5.7.3 to 5.7.4 (#116)

Bumps [acorn](https://github.com/acornjs/acorn) from 5.7.3 to 5.7.4.
- [Release notes](https://github.com/acornjs/acorn/releases)
- [Commits](acornjs/acorn@5.7.3...5.7.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: 🎸 add mandatory rss2 attributes on enclosure (#120)

Co-authored-by: Decebal Dobrica <decebal.dobrica@tellimer.com>

* fix: 🐛 atom link needs to point to self (#122)

✅ Closes: 113

Co-authored-by: Decebal Dobrica <decebal.dobrica@tellimer.com>

* Update dependencies + Formatting + Add Comments (#123)

* Update dependencies

* Update formating

* Add comments

* Bump to 4.2.0

Co-authored-by: Karl Ravn <karl.ravn@schibsted.com>
Co-authored-by: Joseph Dykstra <josephdykstra@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jean-Philippe Monette <jpmonette@users.noreply.github.com>
Co-authored-by: mattimbrain <matti.kaivanto@m-brain.com>
Co-authored-by: Jean-Philippe Monette <contact@jpmonette.net>
Co-authored-by: Decebal Dobrica <decebal.dobrica@tellimer.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants