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

Archival Events Support for Tone #20

Open
horahoradev opened this issue May 8, 2022 · 2 comments
Open

Archival Events Support for Tone #20

horahoradev opened this issue May 8, 2022 · 2 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@horahoradev
Copy link
Owner

horahoradev commented May 8, 2022

Context and Motivation

Recently, archival events were added to give the user greater visibility into the state of their archives. For now, these messages provide indications about when videos are scheduled for download, and then a message indicating whether they've succeeded or failed. That's fine, but it's ugly, and a poor approximation of the original design intention: https://raw.githubusercontent.com/horahoradev/horahora-designs/master/Archives_1.png

As a step forward, it would be nice to add a "Tone" field to each archival event, which would indicate the nature of the message.
For example, if a video fails to download, that would be an "Error", and could be colored red on the frontend.
If a video download succeeds, that would be a "Success", and would be colored green. Other messages which are purely informative could be set to "Info", and could be colored white.

Why do any of this on the server instead of the client? We want this to be extensible. What if we want to inform the user if their archived video has hit 100 views? It's subjective, but I think that would be a great future usage for the archival events feature.

Prerequisites

I'd recommend learning about the following topics before approaching this:

  • Golang (going through A Tour of Go is sufficient)
  • GRPC (reading the tutorial is fine, this is a basic usage)
  • SQL
  • React (I used Frontend Masters when I was learning React, but I'm a shitty webdev so do what you feel is best)
  • (optional) docker and docker-compose (the basic tutorials are fine)

Implementation

This feature will need to touch three components:

  • scheduler
  • front_api (the RESTful API)
  • webapp (the react frontend)

Note: the following is an overview, and isn't provided in-order. See the "Ordering" section at the bottom for a suggested execution order.

Scheduler

We'll need to add a new field to the schema of archival_events, indicating the tone of the message. You could either use an int, or an enum, but an int would probably be more extensible. We can define constants within the application itself.

Next, we'll need to change scheduler's events API to add a tone argument: https://github.com/horahoradev/horahora/blob/master/scheduler/internal/models/video_dl_request.go#L63

(it's just one method)

Once that's done, we'll need to change all clients of this method to add a tone argument, e.g. Success, info, or Error. If it compiles, you've changed all client sites correctly.

Lastly, we'll need to change the response protobuf definition for scheduler's "listArchivalEntries" method, here: https://github.com/horahoradev/horahora/blob/master/scheduler/protocol/scheduler.proto#L30

We'll add a new field indicating the tone, regenerate the code, and add the tone to our GRPC response.

First, we'll need to change the fields we're querying for within scheduler: https://github.com/horahoradev/horahora/blob/master/scheduler/internal/models/archive_request_repo.go#L44

Add the new "tone" database field, and change the "Event" struct accordingly (e.g. add a new int64 field named "tone"), then add it to the invocation of scan, here: https://github.com/horahoradev/horahora/blob/master/scheduler/internal/models/archive_request_repo.go#L65

From there, you'll just need to add it to the GRPC response: https://github.com/horahoradev/horahora/blob/master/scheduler/internal/grpc/grpc.go#L86

Front_api

Front_api is the RESTful API that the frontend interacts with.

The route GET /archivalrequests returns both the current request list and any events: https://github.com/horahoradev/horahora/blob/master/front_api/routes/GET_archiverequests.go#L16

All you'll need to do here is change front_api's version of scheduler's protobuf definitions to match the latest version. For some reason, I return scheduler's generated protobuf types directly here as JSON (maybe this should change, but it doesn't particularly matter).

Webapp

Once the above is done, you should be able to login, visit /archivalevents and see the "Tone" field you've added in each event. From here, we'd want to add a new field in the events table indicating the nature of the event. If it's a success, we should have a green checkmark, and if it's an error, we should have a red X (or something).

Ordering

Here's how I'd approach this:

  1. Add the new "Tone" field to scheduler's scheduler.proto file
  2. sudo ./gen_protos.sh (this is a script which will generate the definitions for GRPC)
  3. Add Tone Postgres field as a migration in scheduler
  4. change scheduler's event creation API to use the new tone field
  5. change scheduler's SELECT to query for the new field, add it to the Scan invocation, add a new Tone field to the Events struct definition
  6. Add the Tone event field to the GRPC response within grpc.go
  7. commit, push to master on your fork
  8. go get github.com/<your username>/horahora/scheduler@<latest commit> within front_api to pin its proto definition to the latest
  9. sudo make up, login, archive something, navigate to the archival requests page and see if the network tab shows that you're receiving the new tone field
  10. Use the tone field to make the archival events table look prettier

Notes

I will probably separate the API for getting archival events vs archival requests in the future, but I can still leverage any work that's been done here to add tone to events.

@horahoradev horahoradev added help wanted Extra attention is needed good first issue Good for newcomers labels May 8, 2022
@horahoradev
Copy link
Owner Author

horahoradev commented May 8, 2022

This issue has been assigned to Whiffmerchant. It seems that I can't assign issues to people without having invited them as collaborators, which would give them write access.

@GabenGar
Copy link
Contributor

Just protect dev and master branches from pushes by anyone but you.
Also "tone" is a bit too vague, pretty sure type is more fitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants