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

[HOLD for payment 2023-05-09] [$1000] [ExpensifyHelp] Create routes.yml file automatically based on the article files #17241

Closed
marcochavezf opened this issue Apr 10, 2023 · 43 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement.

Comments

@marcochavezf
Copy link
Contributor

marcochavezf commented Apr 10, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Problem

Adding a new article to Expensify Help requires the author to create a new entry in docs/_data/routes.yml. This helps Jekyll identify the location of the article. However, this approach is susceptible to errors. If there is a typo or if the article is placed incorrectly, it may not appear on the help site.

Why is this important?

Having all articles properly displayed on the Expensify Help site is essential to ensure that users have access to the information they need. If articles are missing, users may be unable to find the help they need to resolve their issues, resulting in frustration and potentially lost time and productivity.

Solution

To minimize errors and ensure that new articles appear on the help site, generate automatically the routes.yml file based on the docs/articles files by a GH action. We should have a new script (probably in .github/actions/javascript) that runs when the site is built and deployed to https://help.expensify.com. Also, we'd need to call that script when the site is built locally (Ideally with a new npm script in package.json)

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016ec7e523d6a42b06
  • Upwork Job ID: 1645535929735892992
  • Last Price Increase: 2023-04-10
@marcochavezf marcochavezf added Daily KSv2 Improvement Item broken or needs improvement. labels Apr 10, 2023
@marcochavezf marcochavezf self-assigned this Apr 10, 2023
@marcochavezf marcochavezf added the External Added to denote the issue can be worked on by a contributor label Apr 10, 2023
@melvin-bot melvin-bot bot changed the title [ExpensifyHelp] Create routes.yml file automatically based on the article files [$1000] [ExpensifyHelp] Create routes.yml file automatically based on the article files Apr 10, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~016ec7e523d6a42b06

@MelvinBot
Copy link

Triggered auto assignment to @muttmuure (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 10, 2023
@MelvinBot
Copy link

Current assignee @marcochavezf is eligible for the External assigner, not assigning anyone new.

@Prince-Mendiratta
Copy link
Contributor

Prince-Mendiratta commented Apr 10, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

When publishing new articles, we add them to docs/articles. In order to display them, we need to add the metadata to docs/_data/routes.yml. This is prone to human error.

What is the root cause of that problem?

We should improve this by automating the update of routes.yml file.

What changes do you think we should make in order to solve the problem?

I propose to create a bash script that will make use of UNIX binaries like grep, basename, read to parse the directories and subdirectories to modify the YAML file.

This will be not be very strict on the YAML file, it will just deal with the existing directories and hubs. For any new hub, it'll automatically add the articles and href but the title, description and icon will be expected to be manually added/updated. This will be notified as a github comment by the action or on the console locally. It will only modify the hubs key and leave the home and anything above hubs untouched.

Our aim would be to synchronise the following keys:
hubs.href, hubs.sections, hubs.sections.href, hubs.sections.articles, hubs.sections.articles.href, hubs.sections.articles.title, hubs.articles.href, hubs.articles.title.

Any other keys would be left untouched. If there are any new hubs.href added, a comment will be shown to the user to let them know that a new hub has been added and the relevant icon and description should be updated.

In order to account for the sections key, we will add a subfolder inside each directory listed in docs/articles. For example, for the current configuration, we will have to move docs/articles/send-money/Request-and-Send-Money.md to docs/articles/send-money/paying-friends/Request-and-Send-Money.md.

If this is not feasible as it would mess with the site configuration, I suggest we can use a workaround. If the file needs to be placed inside a section, we can add that section to the filename instead as an extension and parse it accordingly in the script. The format would be: title.section.md. For example, it would be Request-and-Send-Money.paying-friends.md for the existing file. We can then parse the first section before the . as the title, the middle as a section and the last to be the markdown extension. If there is no section, it will be appended directly as an article to the articles key in the yml.

The flow of the script will be as follows:

  1. Read the existing routes.yml file and create an array that will parse the existing YAML configuration.
  2. Read the list of entries inside docs/articles.
  3. Iterate through each entry. If the entry is a folder, it will be marked as an hubs.href key. We expect that each entry here will be a folder. For example, send-money.
  4. For each folder, read the list of entries and iterate through them. If the entry is a file, it will be added as a key to hubs.articles with the respective filename as href and the - removed filename as the title with the extension removed for both. For example, Request-and-Send-Money.
  5. If the entry is a folder, a new hubs.sections key will be created for that hubs.href key. The folder name will be added as a key to hubs.sections.href. The folder name in Title case would be added as a key to hubs.sections.title. This will be a new design pattern to account for sections.
  6. For each subfolder inside the parent folder, we will list the files inside here. It is expected that everything inside the subfolder will be a file and no more nesting. We will add each file as a key in the href.sections.articles in a manner similar to step 4.

This will ensure that we are only updating the existing values and not overwriting anything significant.

Although I'd prefer to do this in bash, it would involve logic that requires a certain amount of proper expertise with these tools and might not be friendly to maintain. If you think this might be an issue, I can implement the logic above in Javascript as well.

@marcochavezf
Copy link
Contributor Author

Should I create the code / design for this and propose with that?

Hi @Prince-Mendiratta, yes please, could you provide a more detailed approach to how the script will look like? So we can know if it will work for GH actions and local development. It can be just pseudo-code.

@tienifr
Copy link
Contributor

tienifr commented Apr 11, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Adding a new article to Expensify Help requires the author to create a new entry in docs/_data/routes.yml. This is manual and error prone.

What is the root cause of that problem?

This is a feature request, we should automate the generation of the routes.yml file.

What changes do you think we should make in order to solve the problem?

  1. Add a .github/actions/javascript/generateDocsRoutes/generateDocsRoutes.js script
    This script will:
  • Read the file structure of docs/articles
  • Generate the routes.yml with the following rules
    • The hubs is the top level folders: playbooks, request-money, send-money, ... The folder name is the href of the hub
    • The sections are the second-level folders. The folder name is the href of the section. Currently we don't have that structure but we need to group it that way so that we can automatically generate the sections.
    • We need to add a _index.md to the hub and section folder to store metadata on it (title, description, icon, ...) so we can add it to the generated routes. We can put the metadata in the YAML front matter block of that file. The file has the _ so it's only for our internal use, Jekyll will not process files with _ prefix into its destination site.
    • The articles will be the individual .md file inside hubs and sections folder (excluding _index.md)
    • The article file name will be the href
    • The title and description in the YAML front matter block of the article file will be the title and description in the route definition
    • The home section will be whatever is initially defined in routes.yml, or if we want to be consistent we can create an _index.md file inside the docs/articles folder to store the metadata (href, title, description) for the home block

Based on the rules above we can generate the full routes.yml file based on the articles folder.

  1. Add the generateDocsRoutes to the npm scripts in package.json so we can run it (npm run generateDocsRoutes) during local development

  2. Add the generateDocsRoutes step to the GH actions workflow to build the https://help.expensify.com/ site, we'll run this every time we build.

  3. The routes.yml doesn't need to be committed to the repo since it's a generated file now. If we still want to commit it then there'll be slight changes to the above steps (eg. we'll add GH actions to make sure every PR, that has changes to docs/articles, has updated the routes.yml file properly in order to be merged).

What alternative solutions did you explore? (Optional)

NA

cc @marcochavezf

@pubudu-ranasinghe
Copy link
Contributor

pubudu-ranasinghe commented Apr 13, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

The issue is that when new articles are added to the help site, a new entry must be manually added to _data/routes.yml, which is prone to errors.

What is the root cause of that problem?

The root cause is that updating routes is currently a manual step.

What changes do you think we should make in order to solve the problem?

Instead of using Github Actions, I propose a different approach. Using Github Actions adds a new dependency to the build step and requires a manual step to generate routes when running locally. Fortunately, Jekyll already has features that can support our goal.

  1. Jekyll provides the site.pages variable, which includes details about all the pages on the site. We can use this variable directly in _includes/lhn-template.html liquid to render links to all the pages.
  2. However, site.pages is a flat list that does not include the hierarchy of the pages. It only has the relative path of the page in the folder hierarchy and the page URL. To solve this, we need to transform the list of pages into a tree-like structure so that we can build the navigation using this hierarchy.
  3. To do this, we can write a simple Jekyll generator that builds a tree using the URL path hierarchy, given a list of URLs. We can then add the new tree into a new page variable, such as site.data.navigation
  4. Here's a sample pseudo code:
def build_tree(urls)
  tree = {}

  urls.each do |url|
    parts = url.split('/')
    node = tree

    parts.each do |part|
      node[part] ||= {}
      node = node[part]
    end
  end

  tree
end

###
tree = build_tree(urls)
site.data.navigation = tree
  1. After that, we can update hn-template.html to read the pages from the new variable and generate the navigation.
  2. However, since we're publishing directly to Github Pages on push, the Jekyll gem has been replaced with github-pages. This plugin silently ignores custom generators or plugins since it will be running on Github servers. To work around this, we need to switch back to the Jekyll gem and use Github Actions for publishing instead of auto-published pages. This will give us better control over the build and deploy process.
  3. This is a simple step and is well documented here. It uses actions/checkout, actions/upload-pages-artifact and actions/deply-pages Github Actions

These are the only changes required, and we won't need to depend on a custom script to keep routes.yml updated.

But we still ended up with a new Github Action?

Yes, but there's an important distinction to make. Adding a Github Action to generate routes.yml is a build-time dependency. Unless this step is executed (either in Github Actions or by running the script locally), Jekyll won't output a complete site from its build. In this solution, everything is contained within the build step, and Jekyll builds a complete site. We are only updating the deployment phase to use Github Actions instead of auto-published pages.

What alternative solutions did you explore? (Optional)

Writing a script to update routes.yml and using Github actions for this as suggested in the issue

@melvin-bot melvin-bot bot added the Overdue label Apr 13, 2023
@MelvinBot
Copy link

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

@MelvinBot
Copy link

@marcochavezf, @rushatgabhane, @muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

@muttmuure
Copy link
Contributor

Not blocked, still in discussion

@melvin-bot melvin-bot bot removed the Overdue label Apr 14, 2023
@rushatgabhane
Copy link
Member

didn't get time yet. hopefully will review today 🤞

@marcochavezf
Copy link
Contributor Author

Hi @rushatgabhane! I'm going to step and leave some comments :)

@marcochavezf
Copy link
Contributor Author

We need to add a index.md to the hub and section folder to store metadata on it (title, description, icon, ...) so we can add it to the generated routes. We can put the metadata in the YAML front matter block of that file.

@tienifr I think we don't want to do this step because Jekyll will process the index.md files as a new HTML file and any user could open them in the generated site, which won't be desired.

@marcochavezf
Copy link
Contributor Author

@pubudu-ranasinghe thanks for the proposal, but we're looking for an agnostic solution and don't depend on Jekyll generators, since we'll eventually migrate the site to our custom markdown processor in the future.

@Prince-Mendiratta
Copy link
Contributor

@marcochavezf proposal updated, kindly have a look :)

@tienifr
Copy link
Contributor

tienifr commented Apr 15, 2023

@tienifr I think we don't want to do this step because Jekyll will process the index.md files as a new HTML file and any user could open them in the generated site, which won't be desired.

@marcochavezf sorry I meant a index.md with a _ prefix (_index.md), it's for our use only, Jekyll will not process files with the _ prefix, I updated the proposal for a bit more clarity.

@marcochavezf
Copy link
Contributor Author

Thanks guys, I'm going to assign @Prince-Mendiratta since he posted the proposal first and mentioned his previous experience with GH actions.

Just a few points:

Read the existing routes.yml file and create an array that will parse the existing YAML configuration.

It will be better to have an initial file like _routes.yml containing static information like hub titles, icons, etc., and generate the routes.yml from that.

In order to account for the sections key, we will add a subfolder inside each directory listed in docs/articles. For example, for the current configuration, we will have to move docs/articles/send-money/Request-and-Send-Money.md to docs/articles/send-money/paying-friends/Request-and-Send-Money.md.

Yes, this is better. Non-technical people will add articles (.md files) and will be better for them to see the articles organized by sub-folders.

Although I'd prefer to do this in bash, it would involve logic that requires a certain amount of proper expertise with these tools and might not be friendly to maintain. If you think this might be an issue, I can implement the logic above in Javascript as well.

Correct, despite we have some bash scripts, it's more common and easier to maintain Javascript scripts. So let's do it in Javascript please.

Thanks!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 15, 2023
@rushatgabhane
Copy link
Member

rushatgabhane commented Apr 28, 2023

thanks for taking care of the crash @Prince-Mendiratta @marcochavezf

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels May 2, 2023
@melvin-bot melvin-bot bot changed the title [$1000] [ExpensifyHelp] Create routes.yml file automatically based on the article files [HOLD for payment 2023-05-09] [$1000] [ExpensifyHelp] Create routes.yml file automatically based on the article files May 2, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label May 2, 2023
@MelvinBot
Copy link

Reviewing label has been removed, please complete the "BugZero Checklist".

@MelvinBot
Copy link

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.8-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-05-09. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 8, 2023
@rushatgabhane
Copy link
Member

rushatgabhane commented May 10, 2023

@melvin-bot melvin-bot bot added the Overdue label May 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 15, 2023

@marcochavezf, @rushatgabhane, @muttmuure, @Prince-Mendiratta Huh... This is 4 days overdue. Who can take care of this?

@muttmuure
Copy link
Contributor

Sorry I was out late last week preparing for a conference. Handling this now

@melvin-bot melvin-bot bot removed the Overdue label May 17, 2023
@muttmuure
Copy link
Contributor

Satish G invited to https://www.upwork.com/jobs/~01eabb047ca27d2576

@Prince-Mendiratta also invited

@Prince-Mendiratta
Copy link
Contributor

Thank you @muttmuure, accepted!

@melvin-bot melvin-bot bot added the Overdue label May 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 22, 2023

@marcochavezf, @rushatgabhane, @muttmuure, @Prince-Mendiratta Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot
Copy link

melvin-bot bot commented May 24, 2023

@marcochavezf, @rushatgabhane, @muttmuure, @Prince-Mendiratta 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@marcochavezf
Copy link
Contributor Author

Hi @muttmuure, I think we can close now this issue, correct?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 25, 2023
@rushatgabhane
Copy link
Member

I think the upwork job still needs to be settled :)

@melvin-bot melvin-bot bot removed the Overdue label May 30, 2023
@muttmuure
Copy link
Contributor

Hiring offer sent

@melvin-bot melvin-bot bot added the Overdue label Jun 5, 2023
@muttmuure
Copy link
Contributor

Everyone is paid

@melvin-bot melvin-bot bot removed the Overdue label Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement.
Projects
None yet
Development

No branches or pull requests

7 participants