Edit school in one-record per page view

Issue #83 closed
Brian Lewis repo owner created an issue

Create a page that allows a school record to be loaded and edited.

Comments (44)

  1. Brian Lewis reporter

    See branch Issue83.

    This is a proof-of-concept, based on the existing School Item page. the Address has been converted to inputs, and phone numbers, email added as inputs as well.

    This builds on all the framework in Pull Request #23, which does the heavy lifting, particularly on the server-side.

    The code required to implement the save in the controller is very simple.

    This is not intended to be merged, but to be

    -- a demonstration of how this works ( restangular and PR 23 framework)

    -- the basis for further discussion on where / how to access this functionality in the framework of the application.

    For the record, this is what the school item screen looks like in the Desktop version:

    Capture.PNG

  2. Ghislain Hachey

    I feel like crying. I lost my whole message because of a bad shortcut I pressed and it's been a long day. So here is a much faster and shorter version of what I wanted to say.

    • I still get the following error. The redirect to login page does not work. I need to click on Login to go there but then I can successfully login and things seem to work (don't know if anything else is effected). I upgraded angular-permission and the new script is loaded in the browser. permission-module-loading-error.PNG

    • I see two places where one can edit: address only data in School item page and other stuff by selecting "Registration mode" from "Show columns" at the top. Is that it?

    Here's my two cents for discussions:

    • Grid editing is nice, but only for convenience (quick convenient edit looking at the records like in an excel spreadsheet, very cool). Still need complete editing capability in the school item page (see below). Did not look at the code yet but I assume much of the grunt work for all this was already done in your pull request #23.

    • All data for a school should be editable in a single location (not having to do this two places). I propose to re-organised the tabs of a school item:

      • First tab for dashboard with key data and charts (e.g. latest survey submitted, chart stats, etc.. but no address)
      • Second tab School data (editable in the same way is it with address now) but nicely categorised (e.g. general, address, etc.) much like the desktop screenshot with polished form daat validation, etc.
      • Other remain tabs as-is for now
      • In the school item edit form make it more clear that data is being saved (the current last saved timestamp is a good start but could had several refinements such as only enabling save button when data is changed, spining icon when data being sent, add a nice successfully saved message when complete, proper error feedback, etc.)
    • I raised some proposed UI enhancement in #51 which relates to this. Particularly the last two bullets of that issue. Putting in edit mode is not the same as showing different set of columns in the grid. I think there should be a button somethere below or above the grid "Edit Mode" where the grid just becomes edit mode. The dropdown to show different columns can still be used but as seperate component doing a different thing. I would also opt to changed its location as indicated in the issue I linked to.

    • Can this framework be used to easily add a School? From the School grid (School List) there should be a button "Add School" next to the suggested "Edit Mode" button above

    • Can this work be used to add School surveys? Under School Item -> Surveys tab a Add Survey button (manually fill in survey online) and/or Import Survey (upload directly the PDF filled form)

    That should be enough to get started on discussions

  3. Brian Lewis reporter

    Please fetch latest Issue 83 branch

    I propose to re-organised the tabs of a school item:

    I like this idea - so the first thing you see on the school record is a summary , then you can tab to the school details which is editable. Couple of options about this - - ability to edit the form is security controlled - must have I would say - if you are able to edit, should the page be editable by default, or have an Edit button that flips into edit mode?

    For the record, the related data about a school in the deskop version ( and so in the data model ) contains these elements as well Capture.PNG

    add a School?

    I have sketched a New School function in the latest, noting that: - it fails server side because it is currently only expecting to update an existng record - TO DO

    • current logic is that a school number may be specified for a new school, but ii it is not, each site has their own local rule for creating a new school number.

    So this needs a bit of thinking about what the REST interface may look like:

    currently POST api/schools/<school no> is an update, we need to deal with:

    • attempt to add, that reuses an existing number - shouldn't overwrite the current record, should fail
    • attempt to add, that has no school number - route api/schools needs to be correctly understood

    I think a differentiation between PUT and POST will be the best way to make this unambiguous.

    I propose then we raise an issue to expand the editable tab of school item as Ghislain has suggested, including the contents as shown in the desktop screen shot.

  4. Brian Lewis reporter

    Putting in edit mode is not the same as showing different set of columns in the grid.

    Each possible set of columns for an entity ( eg. Schools) is defined as a "viewMode" in the set of available viewModes. These viewModes define:

    • a columnSet property, passed to the server to get the appropriate set of data;
    • gridOptions, including columnDefs, corresponding pretty closely to the gridOptions/columnDefs of ui-grid
    • some additional info relating to editability.

    So a view mode may be editable or not editable, and that may be permission-dependant. Within the editable viewMode, some columns may still be read-only.

    Currently, when an editable view mode is invoked, it is immediately in "edit mode". The row-header column is added to the left of the grid.

    So following from Ghislain's idea above I'd suggest :

    • that when an editable view mode is loaded, an Edit Button is displayed to the right of the View Mode selector.
    • the view is loaded in read-only mode
    • if the user select Edit Mode: -- the row header column is added to show the row status -- the editable columns are indicated somehow using CSS.
  5. Ghislain Hachey

    currently POST api/schools/<school no> is an update, we need to deal with: attempt to add, that reuses an existing number - shouldn't overwrite the current record, should fail attempt to add, that has no school number - route api/schools needs to be correctly understood I think a differentiation between PUT and POST will be the best way to make this unambiguous.

    Yes. I think POST to api/schools/<school no> should really be a PUT to remain as RESTful as possible. So effectively, we could have POST to automatically assign the new ID and for the cases where the ID is not automatically defined the creation could be using PUT.

  6. Ghislain Hachey

    I like this idea - so the first thing you see on the school record is a summary , then you can tab to the school details which is editable. Couple of options about this - - ability to edit the form is security controlled - must have I would say - if you are able to edit, should the page be editable by default, or have an Edit button that flips into edit mode?

    I like the idea of edit being security controlled (though I would not consider it priority). So default view is a good looking read-only form. An edit button only showing for people with correct permissions which turns the form fields into editable form inplace. with Save/Cancel, etc.

  7. Ghislain Hachey

    I propose then we raise an issue to expand the editable tab of school item as Ghislain has suggested, including the contents as shown in the desktop screen shot.

    Let's do it.

  8. Ghislain Hachey

    So following from Ghislain's idea above I'd suggest : * that when an editable view mode is loaded, an Edit Button is displayed to the right of the View Mode selector. * the view is loaded in read-only mode * if the user select Edit Mode: -- the row header column is added to show the row status -- the editable columns are indicated somehow using CSS.*

    I think this is already much better. I still don't think any of the viewMode or filters dropdowns belong in the top header but this is a discussion for another time.

  9. Ghislain Hachey

    I've fetched the latest from branch issue83. I see the new button is inside a given school detail. This will have to change. It must be somewhere on the grid (list) page. I can play around with that once I get to work on that branch. It's my understanding the server side code for creation is not yet ready and will be worked on in another branch?

  10. Brian Lewis reporter

    i have a branch Issue 87 locally that I will push, PR and merge today. This gives all the server side structure for school create and update.

    We can push back the decision on how to access the Add function by agreeing on the structure of the client side ui-route and its associated clientside (#/) Url.

    So you can access and test your development by navigating to that Url, without access through the menu for now.

  11. Ghislain Hachey

    feat(SchoolItem.cshtml): school edit page

    Includes: - cleanup HTML - improve styling - add new Dashboard tab for school item - add new School Info tab for the school edit page

    Closes #83, #85

    → <<cset 6c74f308cd71>>

  12. Ghislain Hachey

    OK. Have a look at Issue83. And by the way, can we standardise on either issueX or IssueX (I just realised you were capitalising Issue and I not). I like to keep branches like issueX or quick-bug-fix-with-no-issue naming convention.

  13. Ghislain Hachey

    You'll notice a few fields might need some fixups. Especially the confusing District, Islands, Province, etc. Missing lookups as well. But have a look anyway and let me know what you think. Then tomorrow I will consolidate that with your pull request #27. would be good to have something for CRUD'ing school relatively polished by tomorrow. Then I will start looking at teacher. I'll also push the docs branch once we get the School CRUD stuff out of the way.

  14. Brian Lewis reporter

    Changes now required to SchoolItem after merge of issue89:

    1) Trim the content

    Capture.PNG

    The master page now supplies the Panel - so trim the content down to include only the tabset that is included inside this panel

    2) Header

    The master template has placeholders for icon and title. Define these in this razor syntax:

    @section icon {
      fa fa-institution
    }
    @section title {
      {{vm.isNew?'New School': 'School ' + vm.model.schNo + ': ' + vm.model.schName }}
    }
    
        <uib-tabset type="pills">
    

    3) Model references

    SchoolController is replaced by the more general EditController. All references to fields in vm.School should now refer to vm.model

    4) Editable state

    The master layout page can toggle between editable and non-editable views (see panel title bar).

    Inputs should be marked with ng-disabled="!vm.isEditing"

    Inputs that should only be edited on a new record should be marked with

    ng-disabled="!vm.isNew"

    Capture2.PNG

    5) IsNew

    School.isNew is gone. vm.isNew indicates that a new record is being edited.

    6) Vocab

    Labels for fields can be localised using the vocab lookup and its corresponding angular filter.

    Make these replacements:

    EMIS No => {{"School No" | vocab }}

    Also use vocab for

    Island District (called region currently) ElectorateL ElectorateN

    7) District is read-only

    As noted, District is not an editable field on school - it is derived from Island. Display the district name under the island using chained lookup filters as previously demonstrated.

  15. Brian Lewis reporter

    8) Add Another?

    This confirm() is just a placeholder - would be nicer to have a modal here - perhaps invoked through a method of ApiUi.ts where the error reporting dialog are.

  16. Ghislain Hachey

    Yeah, I really don't like the resulting panel within a panel UI. I will try to eliminate that while maintaining all other aspects of template inheritence. I'll get started on that right now. It should not be too long.

  17. Ghislain Hachey

    Created issue #92 for the following. Not as high priority.

    8) Add Another?

    This confirm() is just a placeholder - would be nicer to have a modal here - perhaps invoked through a method of ApiUi.ts where the >error reporting dialog are.

  18. Ghislain Hachey

    Almost there. Saving cause error on backend. I'm moving along for now but here is the screenshot (modal no proper error message) and fiddler following by email (I don't think it's very secure to exchange fiddler tokens through issue tracking system which may eventually become public).

    savings-school.PNG

  19. Ghislain Hachey

    I've pushed local issue83 to origin/isue83 if you want to see progress. Still a few things I want to do such as lookups from #91 and I don't seem to get the expected behavior with the readonly District Name (i.e. Province with my configure data). Also may see saving does not work.

  20. Brian Lewis reporter

    Looking good and like your much-improved Close icon on the panel.

    First thing - I see you changed the form name from schoolForm to vm.modelForm. The new controller EditController.ts is expecting it to be called vm.editForm. But - I think I like modelForm better, so can you edit EditController.ts to change editForm to modelForm.

    When you make this fix, and the name of the form aligns with the variable name used in the controller, then you'll see the form return to locked mode (isEditing = false) after Save or Undo.

  21. Brian Lewis reporter

    On new form, placeholder for schNo doesn;t work

    placeholder="{{"School No" | vocab }}"
    

    Maybe try

    placeholder="{{'School No' | vocab }}"
    
  22. Brian Lewis reporter

    You can allow registration number and registration status date to be editable, even on an exisitng record. In the Sols, registration is a certification process carried out on the school, as it moves through the status values up to Registered, the date should be set as it reaches each new milestone.

  23. Brian Lewis reporter

    Noted about vocab Island - i think this value was added as a vocab later than the database you have, never an issue in the pacific , only added for Africa.

    I will raise an issue to that when a vocab lookup fails, it returns the original search term.

  24. Brian Lewis reporter

    You're right - the errorReporter is not handling this well, probably also should be another issue.

  25. Brian Lewis reporter

    suggestion - on entering the form with a new record

    ie navigating to #/schools/new

    is it possible to focus on to School Info tab, and hide the others?

    Playing with this briefly, it seems possible to hide a tab and its associated data div with

    ng-show="!vm.isNew"

    but, hiding the other tabs won't automatically make the Info tab active:

    Capture.PNG

    It seems that you can use the active property on the tab for that - but this is a two-way binding so cannot not be bound directly to vm.isNew, which would then get turned on whenever the info page is active.

    ....

    Actually playing further I have a solution for this, but let's finish the above and do this later.

  26. Brian Lewis reporter

    Please merge develop to pick up issue93 (vocab filter change). Then you can add vocab-driven labels for Island (ie {{"Island" | vocab }}) and School Class

  27. Brian Lewis reporter

    Take your point about security when the repo goes public. But perhaps there's no need to stress too much about bearer tokens - what you see in the screen dump is only the first 40 or so characters out of > 200. But I'll note to black it out before posting an image anyway.

  28. Ghislain Hachey

    Well this should have been done a while ago. But my git pull --rebase to cleanup history got me into a mess. This is because of two previous merges you did in issue83. It's fine, I just reverted back and did a simple merge of develop into issue83 (like the previous ones you did). I like it less like that cause it makes the history not so pretty but fine for now.

    So I think I am done with this now. Pushed to origin/issue83 if you want to take a look.

  29. Ghislain Hachey

    I did a small cleanup on origin/issue83 commits. I am still getting the database error when trying to save?! Save error I documented previously. Does it work for you?

  30. Ghislain Hachey

    And I am still consistently getting the following error every time I reload the app. Though I can continue with the application?! Does it do that for you? The home page no longer redirects to Login. I must click on signon in the left sidebar. It's been like this since I first started work on issue83.

    permission.PNG

  31. Ghislain Hachey

    Note that I removed all placeholders as they offer nothing but redundancy in the UI in our case.

  32. Brian Lewis reporter

    great work Ghislain, I am being pedantic because I think this is a really important thing to get as perfect as we can make it before we start using it as the standard for other things.

    So if you don't mind I'll dive in could you make a few tiny changes:

    -- I intended for the Save button to be enabled only when the form is dirty ( this fails because there is still a reference to editForm, not modelForm, in the EditPageLayout.cshtml)

    -- I'd like to make the 'waiting' icon, spin - like what you did with Css from the login form

    Make these two changes to the EditPageLayout, commit and make a pull request.

    And we are there.

    Your saving problem is something else... it's working fine for me... is it always the timeout error in sql? I'll raise an issue

  33. Ghislain Hachey

    No worries. Send it back as many times as you want. Sometimes my eyes get tired (lazy). It's done. PR on its way.

  34. Brian Lewis reporter

    feat(SchoolItem.cshtml): school edit page

    Includes: - cleanup HTML - improve styling - add new Dashboard tab for school item - add new School Info tab for the school edit page

    Closes #83, #85

    → <<cset 6c74f308cd71>>

  35. Log in to comment