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

Download Brew Source as Text File #1198

Merged
merged 54 commits into from
Mar 13, 2021
Merged

Conversation

G-Ambatte
Copy link
Collaborator

@G-Ambatte G-Ambatte commented Jan 20, 2021

Download Brew Source

Purpose

The purpose of this branch is to allow users to download the source of a brew as a plain text file.

Work Completed So Far

  • Create code module to output Brew source as a text file
  • Add a new page (download) to get the Brew source using its ID
  • Incorporate the "Download" option into nav bar on Share page
  • Incorporate download option in brew items on User page

Considerations

  • Lint/CircleCi - The 200 line maximum requires that the code for the new page is moved to a new module, which I have called sourceFunctions.jsx in the sharePage directory. It seemed logical to also move the code for the existing "source" page here as well, as large portions of the two functions are identical.
  • "Source" vs "Download Source" in Share page nav bar - I have changed the button text from "Source" to "View Source", to better differentiate the function of the two buttons.

Conclusion

This functionality has been requested previously (e.g. #1129, or this workaround in JS from Reddit). This also goes some way towards a user being able to download all of their brews at once, either as several text files or a single ZIP archive.

It's probably not the cleanest code I've ever written and it's well past midnight so I'm doing this with only one eye opened as I write this PR, so I'd appreciate any feedback that you might have.
-- G


EDIT: These changes should also fix the issue reported with the unmodified ampersand in the HTML source as reported in issue #1188. I added the check and replacement strings as a key/value array, so it should simplify adding any further items to be replaced in the future, if any more should be discovered.

Add detailed MongoDB Install instructions
Eliminate requirement to CD into project directory prior to running `npm start` or `node server.js`.
Adds a RC.d daemon script to control the HomeBrewery status. Based on Andrew Pearson's Node-RED script for the same purpose.
Switch to latest version of MongoDB
Add config items and default values:
- web_port (8001)
- environment (local)
Remove environment variables from rc.d as they are now in config/default.json.
Change to setting NODE_ENV; default to 'local' if not set via environment variable or in the config file.
Remove --force option from `npm audit fix`. While this has not caused any issues to date, there is no guarantee that it will continue to be the case.
Initial write up of install instructions. Includes instruction to `wget` from the `naturalcrit/homebrewery` project rather than the FreeBSD fork, on the assumption that the PR will be merged at some point.
Change to main project repo, on assumption that the PR will be merged at some point
Change of install directory to `/usr/local/homebrewery`
Add `dev_mode` to the HomeBrewery service, which starts the HomeBrewery project in live rebuild mode.
Eliminating unnecessary debugging code, reducing line count and making lint happy :)
Lint is happy, now attempting to pacify CircleCI
Change default port assignment from 8000 to 8001 in config.json and FreeBSD service config
Change to enable use of rcvars for NODE_ENV and PORT
Switch to latest version of MongoDB
Add config items and default values:
- web_port (8001)
- environment (local)
Remove environment variables from rc.d as they are now in config/default.json.
Change to setting NODE_ENV; default to 'local' if not set via environment variable or in the config file.
Remove --force option from `npm audit fix`. While this has not caused any issues to date, there is no guarantee that it will continue to be the case.
Change to main project repo, on assumption that the PR will be merged at some point
Change of install directory to `/usr/local/homebrewery`
Add `dev_mode` to the HomeBrewery service, which starts the HomeBrewery project in live rebuild mode.
… the brew for the source page.

Moved `source` page generation function to a new function module. Added option to function to create plain text download with a sanitized filename and made it accessible from a new page: `download`.
Added the `download` item to the BrewItem so it appears on each brew on the User page.
Added `sanitize-filename` dependency to `package.json`.
Remove new function from `Homebrew.Model.js`
Remove `sourceFunctions.jsx` module
@G-Ambatte
Copy link
Collaborator Author

Shifted the functions back into server.js and removed both the unnecessary changes to Homebrew.Model.js and the new module. Only four files are now affected by this change, at least two of which are trivial changes.
At this point, I haven't got a working NaturalCrit login system yet, so I've been unable to test with a logged in user yet, and by extension, any of the GoogleActions.

  • Test functionality with Google brews

@calculuschild calculuschild added this to the v2.11.0 milestone Jan 24, 2021
@G-Ambatte
Copy link
Collaborator Author

  • Test functionality with Google brews

Login system now working, including Google linking. Download confirmed working identically for Google-stored brews as it does for locally stored brews.

Discussion points:

  • Download filename prefix: Prefixing the filename with "Homebrewery-" makes for quite a long filename. Would this feature benefit from a shorter filename prefix, e.g. "HB-"?
  • Other issues: What else have I forgotten to consider?

@G-Ambatte
Copy link
Collaborator Author

/download appears to be working correctly in all respects, for all brews - local and on Google drive.

@calculuschild
Copy link
Member

@G-Ambatte Just need the config value for const prefix = config.get('name_prefix'); in server.js, since the config file is missing from this PR. Can you remind me what we wanted to use for that?

Gonna try to trim down the Server.js since a lot of stuff is duplicated between HB and Google brews (replacing < with &lt, prepping the file for download, etc). This does reveal a lot of other duplicated stuff in the other URLs as well so I might refactor those as well.

@G-Ambatte
Copy link
Collaborator Author

@G-Ambatte Just need the config value for const prefix = config.get('name_prefix'); in server.js, since the config file is missing from this PR. Can you remind me what we wanted to use for that?

I was using "Homebrewery' but Gazook89 suggested "HB", which I have no objections to. From memory, I moved it to a config item so it could easily be changed later but also so I didn't actually have to make that decision at the time.

@calculuschild
Copy link
Member

Ok. I'm just cleaning up the duplicated functions a bit and then this can merge and go out with v2.11.0

@calculuschild calculuschild changed the base branch from master to PRODUCTION March 13, 2021 03:26
@calculuschild calculuschild changed the base branch from PRODUCTION to master March 13, 2021 03:26
@calculuschild calculuschild merged commit 24957c6 into naturalcrit:master Mar 13, 2021
@G-Ambatte G-Ambatte deleted the sourceDL branch August 8, 2023 00:17
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.

3 participants