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

Refactored schedule tree #169

Closed
wants to merge 26 commits into from
Closed

Conversation

amir-qayyum-khan
Copy link

Background

fixes #146, fixes #165, fixes #54, fixes #39

What is done in this PR

Studio Updates: None.
LMS Updates:

  • Refactored schedule page to use full potential of backbonejs
  • Now schedule tree will not collapse after setting dates or removing child.
Detail:
  • User can see start or due date on schedule tree.
  • User can remove a unit (can be chapter, subsection or vertical) from tree. By doing so we will hide this component along with children by setting `hidden=true``
  • Added validation on changing start or due date on schedule tree.
  • The units with hidden=false is shown to schedule tree on left size, the units with hidden=true is supplied to form at right side. From there user can change visibility of hidden units by clicking Adding a unit or Adding all units
  • Added validation on start and due date in form.
  • In this refactoring I moved possible code to template.underscore files.
  • Made custom view for tree, right container, set date button
  • ccx_schedule.js is the main view. It saves, edit, update collection and renders other view

@pdpinch @giocalitri

screen shot 2016-01-04 at 7 41 22 pm

@amir-qayyum-khan amir-qayyum-khan changed the title Refactored schedule tree WIP Refactored schedule tree Jan 4, 2016
@pdpinch
Copy link
Member

pdpinch commented Jan 4, 2016

@justinabrahms this is the Backbone PR I mentioned in stand-up.

The original JavaScript was done by a contractor who is long gone. DB didn't like it, which was making it hard for us to make modifications.

@justinabrahms
Copy link

@amir-qayyum-khan Can you comment more on what specifically was refactored? I'm not clear what you mean by realizing the "full potential of backbone". I would expect more code deletion as part of this refactoring.


// This data will be render on schedule tree.
this.scheduleTreeData = this.pruned(scheduleJson, function(node) {
return !node.hidden;

Choose a reason for hiding this comment

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

Should this also be excluding on vertical?

var ccxScheduleTreeNode = new scheduleTreeNode.
ccxScheduleTreeNodeView({ model: chapter });
ccxScheduleTreeNode.render();
this.$el.append(ccxScheduleTreeNode.el);

Choose a reason for hiding this comment

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

Instead of manually altering the DOM, consider a template.

@justinabrahms
Copy link

In general, it would be good to add some documentation about what these modules are for and how they're outlined. I found myself wondering how this stuff all fit together.

@amir-qayyum-khan amir-qayyum-khan force-pushed the refactor/aq/schedule.js branch 5 times, most recently from 4dc8c3a to 017a697 Compare January 7, 2016 14:20
@amir-qayyum-khan amir-qayyum-khan changed the title WIP Refactored schedule tree Refactored schedule tree Jan 7, 2016
@amir-qayyum-khan
Copy link
Author

@justinabrahms can you please review ?

Giovanni Di Milia and others added 23 commits February 25, 2016 10:27
…paced-subsection-config

Fix subsection config for self-paced course
…ctoring_0_12_11

Incremented edx-proctoring version to 0.12.11
MA-1919 making mobile handout links accommodate jump to id's and cour…
…eplay)

The get_or_create function is vulnerable to race conditions in MySQL, which can
cause the model LoginFailure to, in some cases, have more than one row for the
same user, breaking the login for that user.

Addinf functionality to expect and clean the error by deleting extra rows (by
oldest lockout date), leaving just one entry and allowing the user to login.

Replayed and squashed by @efischer19, initially commited by @laq
Replayed by @efischer19 due to git issues, initially commited by @laq
Added extra field to CCX model for Course Models
Avoid MultipleObjectsReturned errors with LoginFailures
Syncs preface with edx-doc repo version
@pdpinch pdpinch removed this from the Feb 1 milestone Mar 2, 2016
emZubair pushed a commit that referenced this pull request Sep 30, 2020
SECURITY FIX : Fix CAPA Problems
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants