Skip to content

Conversation

@bgargi
Copy link

@bgargi bgargi commented Jun 5, 2018

Description:
Added feature for exporting a single unit into epub format.
By- Xchange Group BITS Pilani Goa

@kedar2a
Copy link
Contributor

kedar2a commented Jun 5, 2018

👍 for your first pull request (PR).


Adding some suggestion for this PR:

VIEWS/URLS

  • Create new view/url only when there is a new feature/module creation.
  • Possibly try to write code in existing view: export_to_epub.py.
    • This will avoid rewriting of same code.
    • Maintain one copy of code/function, easy to make future changes in logic or policy.
    • Improvise existing function, made it more versatile.

HTML TEMPLATES:

  • Do not write inline css. It is good to start and quick test but not to go in master branch. Either add it in infile or external (always best approach) css/saas file.
  • Check responsiveness of site/page-rendering after adding new CSS.
  • Be careful ( do it only when it's needed) while changing logical conditions within templates.

</a>
</li>
</li>
{% endcomment %}
Copy link
Contributor

Choose a reason for hiding this comment

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

If this code is not needed, remove it.
If needed in near future, add developers note/comments in it.

<a href="{% url 'asset_list' group_name %}" {% if title == 'asset_list' or title == 'asset_detail' or title == 'asset_content_detail'%}class="selected"{% endif %}><i class="fi-folder-lock"></i> {%trans "Assets" %}</a>
</li>
{% if group_object.agency_type != 'School' and "Author" not in group_object_member_of_names_list %}
{% if group_object.agency_type != 'School' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

While making changes in any of the logical conditions, test it with following use users:

  1. Anonymous
  2. Normal
  3. Group Admin
  4. Super

url(r'^save_metadata', 'save_metadata', name='save_metadata'),
url(r'^save_interactions', 'save_interactions', name='save_interactions'),
url(r'^export_to_epub/(?P<node_id>[\w-]+)', 'export_to_epub', name='export_to_epub'),
url(r'^export_unit_to_epub','export_unit_to_epub',name='export_unit_to_epub'),#Added by Gargi Balasubramaniam
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary because git keeps details for each and every line:

  • creator
  • timestamp
  • commit no
  • commit msg

pass
return HttpResponseRedirect(reverse('unit_detail', kwargs={'group_id': group_id}))


Copy link
Contributor

Choose a reason for hiding this comment

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

Try to use the existing method and make it more versatile. Method behavior should be decided by args.

@bgargi
Copy link
Author

bgargi commented Jul 11, 2018

Have also committed changes for adding 2 docs- Export_moodle and Epub_issues(for script issues which cause epub validity issues).

@gnowgi
Copy link
Member

gnowgi commented Oct 25, 2018

@SheetalKashid please test this and let me know the test results. This is an important and useful feature, so should be merged in the master.

@gnowgi gnowgi closed this Oct 25, 2018
@gnowgi gnowgi reopened this Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants