DEV Community

loading...

This Horrifying Force (The Desire To Merge)

chrispinkney profile image Chris Pinkney ・4 min read

​ The weekend crept around like a bad habit. I spent it working at my job and making progress on my two PRs which couldn't be landed in our Friday release due to an acute programming deficiency time and review constraints. Both PRs went through a rampant series of reviews before they could be landed, which I am consistently and eternally grateful towards. Telescope would look like a dog's breakfast were I put in charge of approving or disapproving code integrations. Plus it's always good to have someone better than you review your code:

Alt Text

​ But we'll get into the users micro-service stuff shortly. Switching gears to some Telescope maintainer work, I approved a PR which adds Portainer, a container management GUI (which I had explored a few days prior with Josue) which makes the Telescope admin's lives much easier. I used to host a lot of game servers when I was younger (still do) so I love to play around with software like this.

I also approved a small PR which fixes a typo we had in our CSS, and another fixing the way we display hyphens in Telescope users names. The latter PR had some pretty interesting discussion based around how to approach this issue, how should we display long names which contain hyphens? Should we break the name onto a newline, or split the name at the hyphen? Eventually we decided to simply split onto a newline after encountering a hyphen.

I suggested the use of Formik for our new user sign-up page PR. The sign-up page will eventually be sending post requests to the Users microservice to allow Seneca Students to create accounts and add their blog feeds/GitHub info to their profiles. Naturally it's imperative that we employ double-validation (a word which I totally didn't create just now), meaning we should be validating information that users can input in both the frontend and the backend.

I also lead our weekly triage meeting, with my co-pilot Royce. I think it went really well. I certainly didn't feel as nervous as I did the first time leading.


Users Microservice

​ One change of note to one of my PRs mentioned above, was an idea suggested by Head Wizard Davealdore to validate the query parameters used in the users micro-service even further. We currently have code in the Users microservice which parses each parameter passed in when executing a GET request to /?per_page=xxx&page=yyy to retrieve Telescope users. These values for the parameters (xxx and yyy above) can be between >= 1 and <= 100 for per_page, and >= 1 forpage:

const parsePerPage = req.query.per_page < 1 ? 100 : Math.min(parseInt(req.query.per_page, 10), 100);
const perPage = !parsePerPage || parsePerPage < 1 ? 100 : parsePerPage;

const parsePage = parseInt(req.query.page, 10);
const page = !parsePage || parsePage < 1 ? 1 : parsePage;
Enter fullscreen mode Exit fullscreen mode

However, it was suggested to instead delegate some heavy lifting to Celebrate, a library we use in the backend as middleware to our routes, such that we don't have to entirely rely on weird hacky code like the above snippet. It looks something like this:

-    [Segments.QUERY]: {
-      per_page: Joi.number(),
-      page: Joi.number(),
+    [Segments.QUERY]: {
+      per_page: Joi.number().integer().min(1).max(100),
+      page: Joi.number().integer().min(1),
Enter fullscreen mode Exit fullscreen mode

Not only does Celebrate now ensure that the parameters can ONLY be numbers (specifically integers), it ensures that they must be integers with a range between 1 and 100 for per_page and >= 1 for page. With this change also comes a much needed pruning of the spaghetti logic above, as since we're validating both page and per_page, we have no need to parse or specify the values, and the code can simply be changed to the follow:

-   const parsePerPage = req.query.per_page < 1 ? 100 : Math.min(parseInt(req.query.per_page, 10), 100);
-   const perPage = !parsePerPage || parsePerPage < 1 ? 100 : parsePerPage;
-
-   const parsePage = parseInt(req.query.page, 10);
-   const page = !parsePage || parsePage < 1 ? 1 : parsePage;
+   const { per_page: perPage, page } = req.query;
Enter fullscreen mode Exit fullscreen mode

Awesome. I feel that every time I work with Celebrate I always come out feeling quite pleased with this library and with myself. When a JS library ups your self esteem you've probably been inside the house for too long. Sigh.

​ I also wanted to spend some time later in the week converting the unit tests in the Users microservice to e2e tests with Doc Josue, however a big PR from Head Wizard Davealdore was put up for review which changed a lot of the background and common files in the users microservice. Because of these changes I had to alter the order of my operations since a lot of files that I'd be touching are going to get changed after said PR lands. Thus I instead spent the night working towards finally landing my paginated PR instead, followed by a review to Davealdore's above mentioned PR. Also I think a certain head wizard needs a break... or a beer, or both, preferably both:

Alt Text

​ I also reviewed a PR about Jest, which added a watch mode to the e2e test runner. Constantly re-running tests was slowly driving me (further) into madness. I actually never thought that this had to be added, and instead figured it was just part of Jest and that I was, instead, being too lazy to google to run it.

This weekend I hope to finally get around to converting the users tests to proper e2e tests, submit more reviews to Dave's large PR, and ideally start working on adding our data to Redis as a cache between Firebase and Telescope. Happy Easter.


Discussion (0)

pic
Editor guide