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

Change html_url to use display_url in API #1218

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

etanttila
Copy link
Contributor

@etanttila etanttila commented Aug 7, 2023

Description

What?

In the API, change the html_url to use the display_url rather than the absolute_url of an object.

Also, slightly clarify and simplify code by using local function build_aplus_api rather than building the url using the request object.

Why?

Exercises are often embedded in chapters, and if that is the case, it's more convenient for the html_url to direct the user to the display_url, which links to the chapter if the exercise is embedded.

How?

Change HtmlViewField to use display_url rather than absolute_url.

Fixes Jutut issue #29

Fixes #1217

Testing

Remember to add or update unit tests for new features and changes.

What type of test did you run?

  • Accessibility test using the WAVE extension.
  • Django unit tests.
  • Selenium tests.
  • Other test. (Add a description below)
  • Manual testing.

Tested locally that the field gets the correct value in the API. Also tested locally that Jutut works correctly (possible to respond to feedback, etc.) and that the link does in fact direct the chapter when the feedback questionnaire is embedded.

Did you test the changes in

  • Chrome
  • Firefox
  • This pull request cannot be tested in the browser.

Think of what is affected by these changes and could become broken

Translation

Programming style

  • Did you follow our style guides?
  • Did you use Python type hinting in all functions that you added or edited? (type hints for function parameters and return values)

Have you updated the README or other relevant documentation?

  • documents inside the doc directory.
  • README.md.
  • Aplus Manual.
  • Other documentation (mention below which documentation).

Is it Done?

  • Reviewer has finished the code review
  • After the review, the developer has made changes accordingly
  • Customer/Teacher has accepted the implementation of the feature

Clean up your git commit history before submitting the pull request!

In the API, change the html_url to use display_url rather than
absolute_url. This particularly affects the exercise serializer.

Also, slightly clarify and simplify code by using local function
build_aplus_api rather than building the url using the request object.

A unit test requires the BASE_URL to be overriden so that the tests pass
both when run locally and through the GitHub workflow.

Fixes Jutut issue apluslms#29

Fixes apluslms#1217
Copy link
Contributor

@markkuriekkinen markkuriekkinen left a comment

Choose a reason for hiding this comment

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

Looks good!

@markkuriekkinen markkuriekkinen self-assigned this Aug 8, 2023
@markkuriekkinen markkuriekkinen merged commit 221e52d into apluslms:master Aug 8, 2023
8 checks passed
@etanttila etanttila deleted the api-html-url branch August 8, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants