Bug(partition-view): two implementation details are fragile

[erikmd]

Related project scope(s): partition-view

Bug description

Regarding learnocaml_partition_view.ml, it seems two points would deserve to be improved after fixing #438:

1. learn-ocaml/src/state/learnocaml_api.ml

   Lines 370 to 372 in fbf3125

   	| Partition (token, eid, fid, prof) ->
   	get ~token
   	["partition"; eid; fid; string_of_int prof]

   
   → this basically turn Partition("X-U1O-AB1-1FJ-ZOF", "demo", "times", 30) into an HTTP request to
   http://localhost:8080/partition/demo/times/30?token=X-U1O-AB1-1FJ-ZOF
   → I believe this is an issue if the exercise_id contains a /, e.g.: "fpottier/alpha_beta"
   → the URL should be changed to something like
   http://localhost:8080/partition/fpottier%2Falpha_beta/times/30?token=X-U1O-AB1-1FJ-ZOF
   or http://localhost:8080/partition/times/30?id=fpottier/alpha_beta&token=X-U1O-AB1-1FJ-ZOF
   → a bit like the already-existing URL http://localhost:8080/partition-view.html?id=demo&function=times&prof=30
   → Fortunately, changing this wouldn't impact the learn-ocaml-client-related backward-compatibility claim:
   

   learn-ocaml/src/state/learnocaml_api.mli

   Lines 70 to 79 in fbf3125

   	Regarding the inevitable extensions of the API:
   	- make sure we only add constructors to this [request] type,
   	- and that their semantics does not change
   	(or at least in a backward-compatible way;
   	see PR https://github.com/ocaml-sf/learn-ocaml/pull/397
   	for a counter-example);
   	- but if a given entrypoint would need to be removed,
   	add a [Compat.Upto] constraint (*<*) instead.
   	*)

2. learn-ocaml/src/app/learnocaml_partition_view.ml

   Line 34 in fbf3125

   let _win = window_open ("/student-view.html?token="^tok) "_blank" in

   
   → the "BASE_URL" should be prepended, similarly to:
   

   learn-ocaml/static/partition-view.html

   Line 36 in 5ebb02d

   document.getElementById('chamo-img').src = learnocaml_config.baseUrl + '/icons/tryocaml_loading_' + n + '.gif';

   
   (Note to myself: this line of learnocaml_description_main.ml is also impacted ↓)
   

   learn-ocaml/src/app/learnocaml_description_main.ml

   Line 46 in 1f73141

   (Printf.sprintf "/description/%s#%s=%s"

Current configuration

• OS name (and version): GNU/Linux
• Browser name (and version): Firefox
• learn-ocaml --version: 0.13.1

[erikmd]

Dear @Plictox,
I believe it would be natural for you to start investigating this bug report #456
as soon as you'll have pushed a first complete version of #548 for review (i.e. hopefully, before our upcoming meeting :)