Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Show user even when add new user is cancel #719

Closed
dinhvle opened this issue Oct 10, 2016 · 10 comments
Closed

Show user even when add new user is cancel #719

dinhvle opened this issue Oct 10, 2016 · 10 comments
Labels
🐛bug issue/pull request that documents/fixes a bug in progress indicates that issue/pull request is currently being worked on

Comments

@dinhvle
Copy link

dinhvle commented Oct 10, 2016

Expected behavior:
Should not show user when add user is cancel

Actual behavior:
User is being shown even if you cancel add user

Steps to reproduce:

  1. Run HR locally with CouchDB v1.6.1
  2. Log in as System Administrator
  3. Go to Administrator > Users
  4. Click + new user and cancel it
  5. See a new user from User Listing

Screenshots (if applicable):
screen shot 2016-10-10 at 11 39 15 am

OS and Browser:
OS X 10.11.6
Chrome Version 53.0.2785.143 (64-bit)

@kartik95
Copy link
Contributor

Hi, I would like to take up this issue. @tangollama

@jkleinsc jkleinsc added 🐛bug issue/pull request that documents/fixes a bug Admin in progress indicates that issue/pull request is currently being worked on labels Oct 11, 2016
@jkleinsc
Copy link
Member

@kartik95 It's yours. Thanks for pitching in!

@kartik95
Copy link
Contributor

kartik95 commented Oct 12, 2016

I have a doubt, since documentation is not present in each file (which I would strongly recommend to have). In the 'edit-panel.hbs' component, what is the difference between cancelButtonText and cancel? Anyways, both are performing the same action 'cancel'.

screenshot from 2016-10-12 17 27 27

Also, can anyone make me go through the workflow? It is kind of difficult to get it without the documentation.

@gnowoel
Copy link
Contributor

gnowoel commented Oct 12, 2016

@kartik95 Perhaps we could fix the problem in either of the two ways:

  • only displaying the persistent records in the index action, or
  • pruning unsaved records when leaving the new action.

My personal preference is the latter, which is removing the unsaved new records. But I can be wrong since I'm also new to Ember.

@jkleinsc
Copy link
Member

@kartik95 the reason for the two is that you can override what text appears for the cancel button on an edit screen by setting the cancelButtonText in your controller. In the case of the edit controller for the user edit screen, it is extending app/controllers/abstract-edit-controller which defines cancelButtonText as a computed property that displays either Cancel or Return depending on the state of the data model.

As far as this issue is concerned, I took a look at it and it turns out it is an issue with a with edit-panel component. I'll unpack it below.

@jkleinsc
Copy link
Member

@kartik95 So here is what is happening. I'll try to go into some detail, so forgive me if some of this information is obvious. The user edit route is defined here:
https://github.com/HospitalRun/hospitalrun-frontend/tree/master/app/users/edit

If you take a look at the template for this route:
https://github.com/HospitalRun/hospitalrun-frontend/blob/master/app/users/edit/template.hbs

You will see that the whole thing is wrapped in a component, edit-panel. That component defines the buttons and corresponding actions to fire when clicking on those buttons. So if we look at the code for the edit-panel component here: https://github.com/HospitalRun/hospitalrun-frontend/blob/master/app/components/edit-panel.js
Here is the code for the cancel action:

    cancel: function() {
      this.sendAction('editPanelProps.cancelAction');
    },

So what ends up happening is that editPanelProps.cancelAction fires - but what is that?
It turns out that the controller for the user edit/add extends:
https://github.com/HospitalRun/hospitalrun-frontend/blob/master/app/controllers/abstract-edit-controller.js
That controller includes a mixin:
https://github.com/HospitalRun/hospitalrun-frontend/blob/master/app/mixins/edit-panel-props.js
This mixin basically pulls attributes off of the controller and packages them up in an object to pass along to the edit-panel component.

The abstract-edit-controller defines cancelAction as cancelAction: 'allItems', and that is what ends up getting passed along to the edit-panel component. allItems goes back to the index page, but the problem is a rollback on the model should happen before that action fires. It turns out that the abstract-edit-controller has an action cancel that does this rollback and then calls the cancelAction, which is what should be called instead of allItems
https://github.com/HospitalRun/hospitalrun-frontend/blob/master/app/controllers/abstract-edit-controller.js#L85
At the root of the problem here is the overloading of cancelAction. The edit-panel component really shouldn't pull in the cancelAction from editPanelProps and in fact editPanelProps shouldn't carry cancelAction in it at all. I'll add another comment with the TLDR changes required.

@jkleinsc
Copy link
Member

@kartik95
Copy link
Contributor

kartik95 commented Oct 12, 2016

You pretty much explained the whole issue. Thanks a lot 😄
I'll get to it. Also, will this issue carry a 'hacktoberfest' label?

@kartik95
Copy link
Contributor

Fixed the bug. PR #729

@jkleinsc
Copy link
Member

@kartik95 thanks for contributing. I added the hacktoberfest label.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛bug issue/pull request that documents/fixes a bug in progress indicates that issue/pull request is currently being worked on
Projects
None yet
Development

No branches or pull requests

4 participants