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

Functionality to manually add URL's to the feeds overview #40

Open
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

FokkoVeegens
Copy link
Collaborator

@FokkoVeegens FokkoVeegens commented Jan 8, 2025

This pull request introduces a new GitHub Actions workflow to automate the addition of URLs to a data.json file and updates the front-end to handle and display these URLs. The most important changes include the creation of the workflow file, modifications to the data.json structure, and updates to the front-end logic to merge and display manual URLs.

GitHub Actions Workflow:

  • .github/workflows/add-page-to-feed.yml: Added a new workflow that triggers on issue creation with the label 'add-url', extracts and validates the URL from the issue body, fetches the URL title, updates data.json, and commits the changes. It also comments on the issue and closes it based on the success or failure of the operation.

Data Structure Update:

Front-End Updates:

  • src/pages/Index.jsx: Updated the component to merge manualUrls with existing RSS feed items, ensuring unique display and handling of these URLs. This includes new functions for merging URLs, updating state, and rendering the combined list of items. [1] [2] [3] [4] [5] [6]

These changes collectively enhance the functionality by automating URL additions and integrating them seamlessly into the existing news feed system.

Fixes #32

@rajbos You can test this solution in my forked repo. I provided access for you: https://github.com/FokkoVeegens/github-copilot-updates/issues

@FokkoVeegens FokkoVeegens requested a review from rajbos as a code owner January 8, 2025 17:59
@rajbos
Copy link
Collaborator

rajbos commented Jan 9, 2025

Checking the intent here: do you want to add a single url as a feed item into the data.json and then display it (and then where does it show up?), or do you want to add a complete RSS feed into the data.json and display all items in that feed?

Separate comment: this is rather magic behavior as it is right now, so I would like to add an issue template for this use case, that already fills in the label for the user and then does the rest. But that would be a security issue for people creating issues without write access 🤔. Should be less of an issue then for ourselves.

Perhaps this behavior should be documented then in the issue body of the template, and then the label is something the repo admins can add after reviewing the change. That way we prevent random users of creating issues that end up in our repo 😃.

src/pages/Index.jsx Outdated Show resolved Hide resolved
FokkoVeegens and others added 27 commits January 9, 2025 17:00
@FokkoVeegens FokkoVeegens force-pushed the 32-add-support-to-add-extra-posts-to-a-newsfeed branch from b569bce to 0ab874f Compare January 10, 2025 10:07
@FokkoVeegens
Copy link
Collaborator Author

FokkoVeegens commented Jan 10, 2025

Checking the intent here: do you want to add a single url as a feed item into the data.json and then display it (and then where does it show up?), or do you want to add a complete RSS feed into the data.json and display all items in that feed?

Well, it's your issue, so I could revert the question 😉. I interpreted it as: add a single url as a feed item into the data.json. If that is what you intended, then I guess I implemented it correctly. It shows up amongst the other feed items and can be filtered by clicking the other filter button.

Separate comment: this is rather magic behavior as it is right now, so I would like to add an issue template for this use case, that already fills in the label for the user and then does the rest. But that would be a security issue for people creating issues without write access 🤔. Should be less of an issue then for ourselves.

Perhaps this behavior should be documented then in the issue body of the template, and then the label is something the repo admins can add after reviewing the change. That way we prevent random users of creating issues that end up in our repo 😃.

I can create an issue form for this use case. And update the readme.md with a link to this issue form. Also, for the security of this system, we can route the request based on the author. If it's one of the members of the team, we apply the change directly in the main branch. Otherwise, we'll create a branch and a pull request.

FokkoVeegens and others added 18 commits January 10, 2025 12:26
@FokkoVeegens
Copy link
Collaborator Author

Fixed the issues mentioned in the previous comment. Now when people with write/maintain/admin permissions add a URL, it's applied directly in the default branch (currently a hardcoded branch, needs to be changed). If somebody with read permissions adds a URL, a PR in a separate branch is created.

GitHub Actions Workflow

Documentation Update

  • README.md: Added a section explaining how to add URLs to the newsfeed using the new issue form.

ERROR_MESSAGE="${{ env.ERROR_MESSAGE }}"
ISSUE_NUMBER=${{ github.event.issue.number }}
COMMENT="The addition of the URL is aborted due to the following error: $ERROR_MESSAGE"
gh issue close $ISSUE_NUMBER --comment "$COMMENT"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why close the issue? Adding the comment can help understand why it fails, which gives the requestor the option to update the body and relabel the issue to retrigger the workflow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can do that of course, that would need some refactoring, as the workflow is now only triggered for new issues. I'll add this when I have time again ;-)

README.md Outdated Show resolved Hide resolved
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.

Add support to add extra posts to a newsfeed
2 participants