{"action":"created","issue":{"url":"https://api.github.com/repos/edx/edx-platform/issues/8169","labels_url":"https://api.github.com/repos/edx/edx-platform/issues/8169/labels{/name}","comments_url":"https://api.github.com/repos/edx/edx-platform/issues/8169/comments","events_url":"https://api.github.com/repos/edx/edx-platform/issues/8169/events","html_url":"https://github.com/edx/edx-platform/pull/8169","id":79494292,"number":8169,"title":"TNL-925: Import module amendments","user":{"login":"AkA84","id":6400898,"avatar_url":"https://avatars.githubusercontent.com/u/6400898?v=3","gravatar_id":"","url":"https://api.github.com/users/AkA84","html_url":"https://github.com/AkA84","followers_url":"https://api.github.com/users/AkA84/followers","following_url":"https://api.github.com/users/AkA84/following{/other_user}","gists_url":"https://api.github.com/users/AkA84/gists{/gist_id}","starred_url":"https://api.github.com/users/AkA84/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AkA84/subscriptions","organizations_url":"https://api.github.com/users/AkA84/orgs","repos_url":"https://api.github.com/users/AkA84/repos","events_url":"https://api.github.com/users/AkA84/events{/privacy}","received_events_url":"https://api.github.com/users/AkA84/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/edx/edx-platform/labels/community%20manager%20review","name":"community manager review","color":"207de5"},{"url":"https://api.github.com/repos/edx/edx-platform/labels/open-source-contribution","name":"open-source-contribution","color":"bfe5bf"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2015-05-22T15:21:30Z","updated_at":"2015-05-27T20:10:04Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/edx/edx-platform/pulls/8169","html_url":"https://github.com/edx/edx-platform/pull/8169","diff_url":"https://github.com/edx/edx-platform/pull/8169.diff","patch_url":"https://github.com/edx/edx-platform/pull/8169.patch"},"body":"*(This PR is covered by the OpenCraft contributor agreement)*\r\n*(cc @antoviaque )*\r\n\r\n#### JIRA Ticket\r\n[TNL-925](https://openedx.atlassian.net/browse/TNL-925)\r\n\r\n#### Background\r\nThe current Tools/Import page can be confusing to the user once a course has already been imported: when the user goes back to the page, the page displays the status as if the import had just happened. A quick solution is to display a timestamp (UTC time) of when the last import happened in the \"Success\" step.\r\n\r\nWhile I was working on the ticket I noticed that the `CourseImport` object code and the client script that uses it - *factories/import.js* - could really use a refactoring to improve the readability, maintainability and flexibility of the code. More info on this later.\r\n\r\nI also fixed a couple of UI issue along the way\r\n\r\n#### LMS Updates\r\nNone\r\n\r\n#### Studio updates\r\n**Adding the timestamp**\r\nThe timestamp needs to be in UTC, and I kinda struggled to get reliable results with both `Date.UTC` and [date.js](https://github.com/datejs/Datejs). The dates were either not returned in UTC or they weren't returned in such a format that was not possible to parse and use as a `Date` object. For this reason I decided to turn to [moment.js](http://momentjs.com/), with which I was able to quickly get the desired result.\r\n\r\nThe date is stored as a unix timestamp in the same cookie that holds the filename of the previous/current import file.\r\n\r\n**Miscellaneous fixes to the UI**\r\n* The alert that gets triggered when the user tries to leave the page while uploading the file wasn't using the correct translation function, resulting in the wrong text being displayed;\r\n* The icon in the submit button would get deleted once the label was changed due to a successfull / previous import;\r\n* The gears icon were made spinning by adding via JS the `fa-spin` class, resulting in too much code for something that could've been easily controlled via CSS\r\n\r\n**Code refactoring**\r\nI noticed various issues with the code related to `CourseImport`:\r\n* There was no clear separation of concerns between the object and the client script that uses it. From what I interpreted of the code, the client should be in charge of handling the select/submit/upload process, while `CourseImport` should monitor and give feedback on the current status of the importing process. That was not the case though as both parties were in bits responsible of both areas;\r\n* `CourseImport` interface was a bit lacking, leaking properties and methods that should have been encapsulated behind a better defined API;\r\n* There were instances of duplicated code and methods with implementations too similar to other methods';\r\n* There were unused variable and references to non-existing DOM elements\r\n\r\nSo I tried with the best of my ability to address these issues (the focus was more on `CourseImport` than on `import.js`), and come up with a better refactoring of the code. I think the code has improved, although things could still be better, for example:\r\n* The `updateFeedbackList` method is a bit too long\r\n* The errors (`defaults`) listed in `import.js` I guess should belong to `CourseImport`\r\n\r\nbut I think the PR is in a good enough state now to be reviewed and discussed :)\r\n\r\n#### Screenshots\r\n**Before**\r\n![before](https://cloud.githubusercontent.com/assets/6400898/7773671/5362612e-00a6-11e5-830d-d83deba3751d.png)\r\n![before-alert](https://cloud.githubusercontent.com/assets/6400898/7773669/535afd3a-00a6-11e5-8096-0f263aea8fbd.png)\r\n\r\n**After**\r\n![after](https://cloud.githubusercontent.com/assets/6400898/7773680/612765a2-00a6-11e5-9ed1-4d78dd9ad09f.png)\r\n![after-alert](https://cloud.githubusercontent.com/assets/6400898/7773679/61211e86-00a6-11e5-9b79-1dcad544af26.png)\r\n\r\n#### Testing\r\nThe Import page should still work as expected both in success and error cases.\r\nWhat should be different is that an import has run, a timestamp should appear in the \"Success\" step displaying its timestamp in UTC time, even after a page reload.\r\n\r\nI tested this with both successful and unsuccessful imports and it seems to work as expected (Chrome, FF)"},"comment":{"url":"https://api.github.com/repos/edx/edx-platform/issues/comments/106058288","html_url":"https://github.com/edx/edx-platform/pull/8169#issuecomment-106058288","issue_url":"https://api.github.com/repos/edx/edx-platform/issues/8169","id":106058288,"user":{"login":"AkA84","id":6400898,"avatar_url":"https://avatars.githubusercontent.com/u/6400898?v=3","gravatar_id":"","url":"https://api.github.com/users/AkA84","html_url":"https://github.com/AkA84","followers_url":"https://api.github.com/users/AkA84/followers","following_url":"https://api.github.com/users/AkA84/following{/other_user}","gists_url":"https://api.github.com/users/AkA84/gists{/gist_id}","starred_url":"https://api.github.com/users/AkA84/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AkA84/subscriptions","organizations_url":"https://api.github.com/users/AkA84/orgs","repos_url":"https://api.github.com/users/AkA84/repos","events_url":"https://api.github.com/users/AkA84/events{/privacy}","received_events_url":"https://api.github.com/users/AkA84/received_events","type":"User","site_admin":false},"created_at":"2015-05-27T20:10:04Z","updated_at":"2015-05-27T20:10:04Z","body":"@e-kolpakov well never mind `freezegun`, it obviously doesn't work in acceptance tests for what I wanted to do :)\r\n\r\nSo in the end I settled for two tests: one re the value itself being equal to the current utc time, and the other re its visiblity\r\n\r\nLet me know what you think and if there is anything you want me to add or change, thanks!"}}