maybe-finance / marketing

The marketing site for Maybe
https://maybe.co
GNU Affero General Public License v3.0
17 stars 30 forks source link

Build Stock Portfolio Backtest #122

Closed Shpigford closed 1 week ago

Shpigford commented 3 weeks ago
stock_portfolio_backtest_blank stock_portfolio_backtest

Feature Overview

This issue is for our "Stock Portfolio Backtest" tool which lets the user backtest a portfolio of stocks/etfs and compare performance against another single stock. This will be added as a free mini-tool on Maybe.

Requirements

We want this to be as reusable as possible so future mini-tools are very fast to build, so keep that in mind as you work on this.

This should be as "Rails-native" as possible, making use of Hotwire/Turbo/Stimulus where appropriate.

You can see files/conventions of previous tools: https://github.com/maybe-finance/marketing/tree/main/app/views/tools Relevant Stimulus/Javascript code: https://github.com/maybe-finance/marketing/tree/main/app/javascript/controllers

This tool has a lot of similarities to the Bogleheads Calculator, so probably worth checking out some of that code.

Stock data

You'll pull historical stock data from Synth, which has a free plan for up to 1000 calls.

The Open/Close endpoint is where you'll pull data for each stock. Ex. https://api.synthfinance.com/tickers/VTI/open-close?start_date=2004-06-20&end_date=2012-07-01

Given how many different stocks can be added, we'll need to make a request to Synth each time to the form is submitted, so a loading indicator is important.

Note: You can get a list of all available stocks from the Stock model in the marketing app, so searching for stocks should be very fast as that doesn't require an external call.

Design

Bounty

There is a $500 bounty on this.

To participate in the bounty:

  1. Comment /attempt #122 to indicate you plan on working on this.
  2. Submit a draft PR once you start and include /claim #122 along with your general game plan for how you intend to implement this.
  3. Change out of draft mode on the PR once you're ready to submit your attempt.
  4. Include a short video showing functionality in your PR (drastically speeds up our approval).

In the event of multiple attempts, we generally pick the one that most closely matches the scope with the cleanest code and test coverage (when appropriate).

Shpigford commented 3 weeks ago

/bounty $500

algora-pbc[bot] commented 3 weeks ago

πŸ’Ž $500 bounty β€’ Maybe

Steps to solve:

  1. Start working: Comment /attempt #122 with your implementation plan
  2. Submit work: Create a pull request including /claim #122 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to maybe-finance/marketing!

Add a bounty β€’ Share on socials

Attempt Started (GMT+0) Solution
🟒 @neo773 Aug 21, 2024, 6:55:55 PM #123
🟒 @PlanetKumbhaj Aug 22, 2024, 10:47:54 AM WIP
πŸ”΄ @josefarias Aug 23, 2024, 9:38:05 PM WIP
neo773 commented 3 weeks ago

/attempt #122

Algora profile Completed bounties Tech Active attempts Options
@neo773    5 maybe-finance bounties
+ 124 bounties from 24 projects
TypeScript, Rust,
Go & more
Cancel attempt
Shpigford commented 3 weeks ago

(Assigning to @neo773 due to their consistent track-record with Maybe bounties.)

thekumbhaj commented 3 weeks ago

/attempt #122

josefarias commented 3 weeks ago

@Shpigford is this open for parallel submissions, with the understanding that @neo773's would be preferred if equivalent?

Totally fair if you'd rather only have their submission. This just looks like a fun exercise and I haven't touched Maybe in a while.

Shpigford commented 3 weeks ago

@Shpigford is this open for parallel submissions, with the understanding that @neo773's would be preferred if equivalent?

Totally fair if you'd rather only have their submission. This just looks like a fun exercise and I haven't touched Maybe in a while.

Yep, that's fine. I think @neo773 has at least tackled the frontend but not sure where they are on the backend parts.

josefarias commented 3 weeks ago

/attempt #122

Gotta leave in a few minutes. I'll finish this tomorrow. Will comment here if plans change.

Algora profile Completed bounties Tech Active attempts Options
@josefarias 4 maybe-finance bounties
Ruby, CoffeeScript,
TypeScript & more
Cancel attempt
josefarias commented 3 weeks ago

Won't have time for this today in the end, canceling my attempt! Sorry about the noise.

@Shpigford, I'll say (unsolicited, sorry if I'm overstepping) you're straying pretty far from the Hotwire happy path with these tools. Only just noticed as I was getting familiar with the code for this issue.

Using FormData to parse form fields and using TemplateRenderer to render isn't idiomatic, and this is the first Rails codebase I've seen doing that. I think it makes the code more complicated than it has to be, and FWIW played a part in my not participating here since I had a big choice to make in keeping with the established pattern (and fighting my mental model πŸ˜…) or suggesting an idiomatic one.

Totally get if that's not a priority. But thought I'd mention it if you wanted to eventually prioritize a refactor. I think it'd make the codebase more approachable.

Shpigford commented 2 weeks ago

Won't have time for this today in the end, canceling my attempt! Sorry about the noise.

@Shpigford, I'll say (unsolicited, sorry if I'm overstepping) you're straying pretty far from the Hotwire happy path with these tools. Only just noticed as I was getting familiar with the code for this issue.

Using FormData to parse form fields and using TemplateRenderer to render isn't idiomatic, and this is the first Rails codebase I've seen doing that. I think it makes the code more complicated than it has to be, and FWIW played a part in my not participating here since I had a big choice to make in keeping with the established pattern (and fighting my mental model πŸ˜…) or suggesting an idiomatic one.

Totally get if that's not a priority. But thought I'd mention it if you wanted to eventually prioritize a refactor. I think it'd make the codebase more approachable.

Feedback always welcome! I believe the thinking behind FormData + TemplateRenderer was essentially an attempt at making small form components more useable, but I may be off on that.

I think @JoshAntBrown may have been the one to initially implement that so maybe he can speak more to it?

JoshAntBrown commented 2 weeks ago

Hey, yeah, that was me. I agree with what @josefarias posted though, we've probably drifted a bit too far from the Hotwire happy path.

That first tool I built was pretty basic, and I wasn't sure how we might integrate with the CMS / Rails, so I thought it might be beneficial to keep things mostly front-end driven. With the option of changing it up later should the tools get more complex.

Unfortunately things came up and I didn't get the chance to build any more of the tools, and looks like that initial pattern stuck and evolved from there.

That said, each tool is fairly independent so I don't think there's any reason we should be tied to FormData + TemplateRenderer for all tools. The main thing will probably be changing up how the routing / controllers work for the tools.

Shpigford commented 2 weeks ago

@josefarias Would you have any interest/availability in tackling a "refactor" bounty?

josefarias commented 2 weeks ago

@josefarias Would you have any interest/availability in tackling a "refactor" bounty?

Yeah, happy to take that on. I can identify the following issues after a quick pass:

  1. Lack of Hotwire primitives: use frames, and maybe streams, instead of FormData + <template> elements
  2. Single-use Stimulus controllers: These should be built to be reusable. But that's not possible if they're tied to their specific tool. So I'd move all calculations to the server, and use these for chart rendering only. Hopefully landing on a single controller for each kind of chart (so one configurable controller for all trendlines - which this can share with the Maybe app).
  3. Busy controllers: we're doing too much here, and not in very railsy ways. We'd benefit from extracting rich domain objects that know how to fetch and cache data from Synth for each tool – which should likely also have their own ruby representation to perform calculations.

Some notes:

  • For #2, I already built a good starting point for the app a while ago. But I'll have to modify to support more than one trendline, and clean it up while I'm there.
  • #2 will require some time as I see lots of procedural JS that'll be hard to dissect.
  • We could componentize our HTML a bit better but it's not too bad, I'd tack that on as the other bits are cleaned up
  • You can be sure I won't pull out any abstractions we're not ready for.

Let me know which of these (1-3) you'd like me to tackle. I have a busy week ahead, so I'd probably start the work this weekend.

Feel free to DM with any questions. Contact info's in my GH profile.

neo773 commented 2 weeks ago

I have a differing opinion on this as someone who's primary stack is React/Node background.

Sure the current approach isn't as "Railsy" but it's remarkably simple and doesn't take much time for new contributors to pick up if you know JavaScript/TypeScript.

When I made my first contribution I picked up this pattern in less than one hour and it just works I haven't found limitations to it.

The repository being a marketing frontend benefits from this as it allows you to ship fast and wouldn't benefit much from a refactor to Rails only imo. Anyways that was just my two cents on this this issue.

josefarias commented 2 weeks ago

Sure the current approach isn't as "Railsy" but it's remarkably simple and doesn't take much time for new contributors to pick up if you know JavaScript/TypeScript.

Following Hotwire conventions would be even simpler, and easier to pick up. Because they're documented, and shared across codebases.

The current approach is bespoke and undocumented. I don't think a marketing site is a good place to implement such systems. It invites further customization, further departure from convention, bikeshedding, and locking ourselves in to the contributors who created the system.

Following convention opens us up so anyone familiar with Hotwire can contribute.

The repository being a marketing frontend benefits from this as it allows you to ship fast and wouldn't benefit much from a refactor to Rails only imo.

I'd say let's stick to the chosen stack. This is a Rails & Hotwire app, so we should follow established Rails & Hotwire patterns.

As the saying goes, "if you're not using a framework, you're building one" β€” my recommendation is we should use the framework we chose (which, incidentally, I happen to think is the fastest framework to ship with a small team, but that's subjective.)

EDIT: I'd be remiss not to mention we're conflating the codebase being "Railsy" with my main point. Only #3 above pertains to Rails. I'm mostly advocating for conventions regarding Hotwire, the JS framework.

Shpigford commented 2 weeks ago

I'm going to keep pushing forward with the path on this particular mini-tool and will likely start a new bounty for optimizations/standardization here in the near future.

josefarias commented 2 weeks ago

@Shpigford I have some time this weekend starting today if you wanted to merge the current proposal and immediately start standardizing.

No rush of course, we can do it any time.

algora-pbc[bot] commented 1 week ago

πŸŽ‰πŸŽˆ @neo773 has been awarded $500! 🎈🎊