Together-100Devs/Together

Add an API Endpoint that will return all events

Closed this issue · 8 comments

Related Issues
Project: Admin Dashboard
Depends on:

Description
Add getEventsForAdmin method in the events.js controller that will return all events.

Acceptance Criteria

  • API Endpoint returns all events
  • Tests to confirm API enpoint exists and is functioning properly. This includes creating events, hitting API, and validating what was created is returned.

I think I'd like to try my hand at this issue.
But before I do, could I get some clarification on how this API endpoint should differ from the getAll method that already seems able to return all events?

I think I'd like to try my hand at this issue. But before I do, could I get some clarification on how this API endpoint should differ from the getAll method that already seems able to return all events?

that's a great question. It's been awhile since I checked this, but if memory serves me right, getAll only gets all the events for the current month, but I'll need to check that when I get home. if I'm wrong and getAll is truly a getAll then I'll need to adjust the ticket.

@cblanken

It looks like I was correct that getAll requires a date parameter.

  const response = await DataService.getAll(date.monthStart, date.monthEnd);

  getAll(from, to) {
    return URL.get("/events", { params: { from, to } });
  }

  getAll: async (req, res) => {
    const { from, to } = req.query;

    let where = {};
    if (from || to) where = { startAt: { $gte: from, $lt: to } };

    const query = !req.user
      ? Event.find(where).select("-user")
      : Event.find(where).populate("user", "displayName");

    res.json(await query.lean());
  }

Now, while I guess we could pass in dates from the beginning of time to the end of time, I would still prefer a second endpoint. This is because we may want to change the endpoint only to pass back pending events at some point; additionally having the same endpoint for two entirely different functions does not seem natural.

I'm open to ideas if you think there's a better way to do this! I'll admit I thought a bit about this as work, and there are good reasons both ways.

@Caleb-Cohen
From my tests in Postman, the from and to parameters don't seem to be required.
If you call the base endpoint without them, you'll get all the events.

I don't know the easiest way to add more users while the app is running in development so I haven't tried it with multiple users yet, but here's the Postman response from the getAll endpoint without parameters.

I added two events in addition to the default event added when running in development and they all get returned.

Image

I just realized the code you posted wasn't from server/controllers/events.js. What file is that code snippet from?

I just realized the code you posted wasn't from server/controllers/events.js. What file is that code snippet from?

I pulled the last bit from the controller in events.js, the top bits are from the client side that calls the routes.

I pulled the last bit from the controller in events.js, the top bits are from the client side that calls the routes.

Oh I see. I didn't realize the quote wasn't all one file.

So do we still want to add a separate method (getEventsForAdmin) or would it make more sense to just call the current getAll method with no parameters from the frontend of the Admin Dashboard?

If I'm not mistaken it should return all events if the from and to parameters are left empty (undefined).

I pulled the last bit from the controller in events.js, the top bits are from the client side that calls the routes.

Oh I see. I didn't realize the quote wasn't all one file.

So do we still want to add a separate method (getEventsForAdmin) or would it make more sense to just call the current getAll method with no parameters from the frontend of the Admin Dashboard?

If I'm not mistaken it should return all events if the from and to parameters are left empty (undefined).

Let's use the getAll, we can re-open this ticket if we need a more specialized route in the future!

Thanks for looking into this and correcting my mistake!