Expressjs / Node.js - res.redirect() not loading page

Question

I have a page with a route GET /team which is loading a list of teams, and DEL /team which is deleting a team from /team/:key. So you navigate to a team's profile page and delete them from there, on deletion it should redirect you to the /team page. I have put logs into the console and it is successfully deleting the team and wierdly, it says it is loading /team but the browser does not load this. I have put my code below, any ideas?

Routes:

  app.get('/team'/*, lim("Must be logged in to see teams")*/, getAllTeams, function(req, res){
    util.log('Serving request for url [GET] ' + req.route.path);
    // Pass it the list of all Teams
    res.render('team', {'teamsList' : req.teamsList} );
  });

  app.get('/team/:key', function(req, res) {
    util.log('Serving request for url [GET] ' + req.route.path);
    Team.findByKey(req.params.key, function(err, teamData){
      if(!err && teamData){
        teamData = teamData;
        res.render('teamDetails', { 'teamData' : teamData } );
      } else {
        util.log('Error in fetching Team by key : ' + req.params.key);
        res.json({
          'retStatus' : 'failure',
          'msg' : 'Error in fetching Team by key ' + req.params.key
        });
      }
    });
  });

  /**
    * DEL /team/:key
    * Delete Team by key
    */
  app.del('/team/:key', getAllTeams, function(req, res) {
    util.log('Serving request for url [DEL] ' + req.route.path);    
    Team.remove({key : req.params.key}, function(err){
      var message = '';
      var retStatus = '';
      if (!err) {
        util.log('Successfully deleting Team with key: ' + req.params.key);
        message = 'Successfully deleting Team with key: ' + req.params.key;
        retStatus = 'Success';
        res.redirect('/team');
      } else {
        util.log('Error deleting Team with key: ' + req.params.key + 'Error: ' + util.inspect(err));
        res.json({
          'retStatus' : 'failure',
          'msg' : 'Error in fetching Team with key ' + req.params.key
        });
      }
    });
  });

JavaScript + HTML template:

        button#teamDelete.btn.btn-danger.btn-mini(type="submit", value="Delete Team") Delete
        script(type='text/javascript')
            $('#teamDelete').live('click',function(){ 
                var teamId = #{teamData.key};
                $.post('/team/' + teamId, { _method : 'delete' }, function(response) {
                    console.log(response);
                    if(response.retStatus === 'Success') {
                        if('/team' && '/team' !== "") {
                            window.location = '/team';
                        }
                    }
                });
            });

console logs:

10 Mar 11:52:01 - Serving request for url [GET] /team
10 Mar 11:52:02 - Serving request for url [GET] /team/:key
10 Mar 11:52:03 - Serving request for url [DEL] /team/:key
10 Mar 11:52:03 - Successfully deleting Team with key: 1362855941128
10 Mar 11:52:03 - Serving request for url [GET] /team

getAllTeams:

var getAllTeams = function(req, res, next){
  Team.getAll(function(err, teamsList){
    if(!err && teamsList){
      req.teamsList = teamsList;
      return next();
    }
  });
};

Team.getAll (Team schema)

Team.statics.getAll = function(cb){
    var query = this.find({});
    query.sort({key : -1});
    return query.exec(cb);
  };

Solution 1

Your request is POST ($.post) and you route is app.del, so it never gets to res.redirect inside app.del route.

Why don't you use app.post?

Updated:

Assuming $.post sends HTTP DEL request here what is happening: server sends 302 response with no data but browser never sends another request to GET route as server instructs it (or does jQuery handle redirects too? Not sure). res.redirect() is actual HTTP response not some internal server-side instruction to re-route the request to another route like you can do in ASP.NET (and which is wrong actually)... Route is designed to receive request, reply with the response and forget about it. You need to separate routes from actual functions processing them, then you will be able to call that function instead of sending redirect.

Code suggestions

In app.del('/team/:key' ...

...
    retStatus = 'Success';
    // res.redirect('/team');
    res.send({
      retStatus : retStatus,
      redirectTo: '/team',
      msg : 'Just go there please' // this should help
    });
...

Client-side in $.post('/team/' ...

...
    $.post('/team/' + teamId, { _method : 'delete' }, function(response) {
        console.log(response);
        if(response.retStatus === 'Success') {
            // not sure what did you mean by ('/team' && '/team' !== "")
            // if('/team' && '/team' !== "") {
            if (response.redirectTo && response.msg == 'Just go there please') {
                window.location = response.redirectTo;
            }
        }
    });
...

Not sure it will work though because I don't understand what your getAllTeams does and why you store teamList in req. If you want to store in session, than assuming the middleware is correctly configured you need to use req.session. If you need to store it only within request and your getAllTeams prepares this list of teams it is better to store in res.locals (like res.locals.teamList). And make sure your getAllTeams calls next. So basically your getAllTeams should look like this:

function getAllTeams (req, res, next) {
    res.locals.teamList = [/* whatever */];
    next();
}

And then you can use res.locals.teamList in your route handler instead of req.teamList.

res.render('team', {teamsList : res.locals.teamsList} );

And 'team' template also can have a problem...

Express advice :)

Also the way you use Express makes it very difficult to extend/manage application. I don't remember where exactly, but somewhere in docs they write that Express is supposed to be used as the foundation for your application framework, not as a complete framework like most PHP frameworks are. It gives you a lot of power and flexibility, but also makes it necessary to think about your application architecture well in advance.

The most powerful feature of express is that you can have any route handled by many route-specific handlers/middlewares passing control to each other via next(). I have a static table that defines which handlers are used on each route allowing to see the whole application with 30 or so routes on one page. And this table is used to assemble the routing dynamically when the application starts. It leads to a lot of flexibility, manageability (I can move/copy-paste handlers from route to route - each handler is represented as a single word) and code re-use. I also use the same hash of routes definition in the client for client-side routing.

Solution 2

For a quick workaround, just add the redirect url to the response and on the client side do:

if (redirectUrl && redirectUrl !== "")
    window.location = redirectUrl;

Comments

[germainelol]

So replace every redirectUrl with /team you're saying?

[germainelol]

Still can't get that to work either, can you see if you can see anything wrong with the HTML code?

[Deathspike]

He's using _method which probably has methodOverride on the server.

[germainelol]

I just tried making it app.post rather than app.del and removing the _method: delete from the JS on the HTML page, but it still says routing to /team on console, but nothing loads on the browser. Thanks for the express advice too, I will take this into consideration for sure.

[germainelol]

@RoelvanUden Correct, I am using this.

[Tutan Ramen]

When I go to the console on stackoverflow.com and run window.location = "/hello"; It takes me to stackoverflow.com/hello

[esp]

@RoelvanUden is _method some undocumented jQuery hack? :)

[esp]

@germainelol ok, so it gets to app.del, it send 302 response to the browser which has no data, but browser never sends the new request. Updating the answer...

[esp]

Actually what @TutanRamen wrote will work if you need to display the page, if you need to request the data you have to issue another HTTP GET request ($.get). At least I think so :)

[germainelol]

@esp This is all very confusing to me now :D Are you able to advise me on what I should change with my code? I understand what you are saying but I don't understand how to change the code in order to fix this

[germainelol]

@esp any idea? Would be much appreciated!

[Deathspike]

@esp: _method is a field allowed by the methodOverride middleware supplied by Connect for NodeJS.

[esp]

@RoelvanUden got it, thanks. Although I still don't get why use this middleware at all when you can just use app.post to handle post request... Only if you have some working API used by say mobile app and you don't want to use DEL in browser, but not when you are developing from scratch...

[germainelol]

Yes, your solution there with getAllTeams fixed it :)