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

Redirecting to previous page after login(#105) #119

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anjali142
Copy link
Collaborator

@anjali142 anjali142 commented Jun 24, 2019

What's this PR do? (related issue if exists)

Issue #105
Changes are done in auth.js file of the controls section
Changes are done in order to redirect to previous page after login action

Any context you want to provide on the implementation?

express-back uses sessions to track the previous path the user visits
res.redirect('back') redirects to URL of the previous page that the express-back gives

Fixes: #105

Issue KamandPrompt#105
Changes are done in auth.js file of the controls section
Changes are done in order to redirect to previous page after login action

express-back uses sessions to track the previous path the user visits 
res.redirect('back') redirects to URL of the previous page that the express-back gives

Fixes: KamandPrompt#105
@kpbot kpbot added size: XS enhancement New feature or request labels Jun 24, 2019
@aashish-ak
Copy link
Member

Good job @anjali142 , but you're adding a new package called express-back for this, avoid using external packages because after their support is gone, it'll highly affect our app, this can be easily done by passing the url in the params and then redirect to that url after logout. Update as soon as you can.

@aashish-ak aashish-ak requested a review from Vishal1541 June 25, 2019 10:30
controls/auth.js Outdated
@@ -2,6 +2,7 @@ var passport = require('passport');
var user = require('../models/users');
var submissions = require('../models/submission');
var moment = require("moment");
var back = require("express-back");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New module should not be added.

@@ -78,7 +79,7 @@ exports.postLogin = function (req, res) {
*/
exports.getLogout = function (req, res) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try getting the current url in the req param and then redirect back to that url after calling the logout function.

Copy link
Member

@Vishal1541 Vishal1541 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anjali142 This doesn't seem to work. Can you check it again and make the necessary changes.

@Vishal1541 Vishal1541 requested a review from aashish-ak June 30, 2019 14:30
@@ -7,6 +7,7 @@ var moment = require("moment");
* route: /user/signup
*/
exports.postSignUp = function (req, res) {
var url = req.url;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

req.url is only supported for requests received through http server
It is not from express
try req.headers.referer
this way you will also get the query parameters

@@ -78,7 +79,7 @@ exports.postLogin = function (req, res) {
*/
exports.getLogout = function (req, res) {
req.logout();
res.redirect('/');
res.redirect('url');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

url instead of 'url'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size: XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After login, redirect to previous page
5 participants