1

I have only limited experience with Javascript promises. The app tracks the availability of volunteer firefighters, they can change their availability status from a mobile app or web portal. The cloud function should execute any time a firefighter updates their status. I want the function to do a full count of all firefighters with status: "Available" and then also count drivers and officers within that group.

Then I will check the end totals and if any of the three values is below a set threshold then a push notification is sent to all users notifying them that numbers are getting low.

The problem I have is that the database call admin.firestore().collection('users').where("status", "==", "Available").get() and the push notification function admin.messaging().send(message) both return promises.

If I don't nest the promises then the messages sends before the availability count is completed (as expected). If I nest the send message within the .then( snapshot => { is get lots of ESLint warnings thrown at me about not nesting promises

What's the best way to implement this? I feel I know what I need to do, I just don't know how to do it.

index.js


const functions = require('firebase-functions');
const admin = require('firebase-admin');
admin.initializeApp();

const cors = require('cors')({ origin: true });

exports.sendPushNotification = functions.firestore
    .document('users/{userId}')
    .onUpdate((snap, context) => {

        // This function checks the number of available firefighters each time any firefighter
        // changes their status.  If the numbers are below the minimums specified then a push
        // notification is sent to all users alerting them of low muster numbers.


        let count = admin.firestore().collection('users').where("status", "==", "Available").get()
                    .then(snapshot => {
                        let Available = 0;
                        let Drivers = 0;
                        let Officers = 0;

                        snapshot.forEach(doc => {
                            if (doc.data().driver) {
                                Drivers++;
                            }

                            if (doc.data().officer) {
                                Officers++
                            }

                            Available++
                        });
                        return {total: Available, drivers: Drivers, officers: Officers}
                    }).catch( (error) => {
                        return console.log(error);
                    })  

            message = {
                topic: "MVFB",
                notification: {
                    title: "Warning Low Numbers",
                    body: `Critial availability - Available: ${count.total} Drivers: ${count.drivers} Officers: ${count.officers}`
                },
                android: {
                    priority: "high",
                    notification: {
                        color: "#FF0000",
                        sound: "emergency.m4a",
                        channel_id: "emergency_message"
                    }
                },
                apns: {
                    payload: {
                        aps: {
                            sound: "Emergency.aiff"
                        }
                    }
                },
            };

            admin.messaging().send(message)
            .then((response) => {
                console.log('Notification sent successfully:',response);
                return true;
            }) 
            .catch((error) => {
                console.log('Notification sent failed:',error);
                return false;
            });

        return true
    });
iShaymus
  • 512
  • 7
  • 26
  • You might want to disable that eslint rule, there is nothing wrong with nesting promises - being able to return other promises from the `then` callback is one of their [major features](https://stackoverflow.com/a/22562045/1048572)! When you [can unnest them](https://stackoverflow.com/a/22000931/1048572) though, it is preferred. – Bergi Oct 31 '19 at 01:39
  • Thanks @Bergi, I definitely understand that I can, I'm just trying to understand some best practices. The fact that it tells me I should not do it makes me think there is a better way I need to learn. – iShaymus Oct 31 '19 at 01:43
  • In short: `getCount().then(count => { return sendMessage(buildMessage(count)) }).then(response => true, error => false)` – Bergi Oct 31 '19 at 01:51
  • Don't disable the eslint rule. Nested promises lead to code that's difficult to read and has more bugs. That's why it's a recommended rule to turn on - it will force you to code better. – Doug Stevenson Oct 31 '19 at 01:53
  • Thanks @DougStevenson indeed I am trying to learn to do it better. Already sending the message caused some unexpected behaviour when nested. What is your suggestion based on the duplicate article you linked. I've tried a `.then(() => {}).then(() => {})` but it also said this was a nested promise. Am I better off returning a function? At what point is the promise completed? Can you give an example? – iShaymus Oct 31 '19 at 03:53
  • The marked duplicate question shows the general pattern for how to remove the nesting of promises. Please follow that. I suggest also fixing the indentation of your code so it's easier to see the nesting. – Doug Stevenson Oct 31 '19 at 10:35
  • And just like the commenter on the accepted answer for the solution it is not working. Either using double `.then` or calling the send message in a return function both still flag as nested promises. The code works. That’s not what the question was about. It’s about achieving the result without nesting. – iShaymus Oct 31 '19 at 10:40

0 Answers0