create_learningpath web service - refs BT#17453 #3367
Conversation
|
Lint errors were found. A patch is also available. Please see the report: https://flintci.io/repositories/1013/analyses/37551 This comment was posted by FlintCI. It can be disabled in the repository settings. |
|
Lint errors were found. A patch is also available. Please see the report: https://flintci.io/repositories/1013/analyses/37554 This comment was posted by FlintCI. It can be disabled in the repository settings. |
| @@ -96,6 +109,14 @@ public function __toString() | |||
| return (string) $this->getUrl(); | |||
| } | |||
|
|
|||
| /** | |||
| * @return Repository\AccessUrlRepository|EntityRepository | |||
There was a problem hiding this comment.
We try not to mix legacy code (Database class) with new code (Entities), better to just use in the code
$repo = Database::getManager()->getRepository('ChamiloCoreBundle:AccessUrl');
main/lp/learnpath.class.php
Outdated
| } | ||
|
|
||
| $file_path = handle_uploaded_document( | ||
| $course = \Chamilo\CoreBundle\Entity\Course::getRepository()->find($course_id); |
There was a problem hiding this comment.
You can use api_get_course_entity($course_id); so you won't need the Course::getRepository() function
main/lp/learnpath.class.php
Outdated
|
|
||
| $file_path = handle_uploaded_document( | ||
| $course = \Chamilo\CoreBundle\Entity\Course::getRepository()->find($course_id); | ||
| $learningPath = CLp::getRepository()->find($this->get_id()); |
There was a problem hiding this comment.
I think you can create a new function api_get_lp_entity($lpId); so you won't need to call the CLp::getRepository()
| */ | ||
| public static function getCurrentCourse() | ||
| { | ||
| return self::getRepository()->find(api_get_course_int_id()); |
There was a problem hiding this comment.
Better not to mix legacy code (from api.lib.php), to this new entity classes...
Partialy simplified and already improved performance of these API functions: api_item_property_update api_max_sort_value CourseManager::create_course AddCourse::register_course CourseManager::fillCourse AddCourse::fill_db_course learnpath::add_lp
|
Lint errors were found. A patch is also available. Please see the report: https://flintci.io/repositories/1013/analyses/37637 This comment was posted by FlintCI. It can be disabled in the repository settings. |
Fatal error: Uncaught PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'c_id' cannot be null
Course::prePersist calls Course::createSettings
|
Ce PR est inclus dans le PR #3372 donc il sera revu directement dans cet autre PR qui contient d'autres Web Services en même temps. |
|
This PR has code already sent in #3372 and it has the same issues (database changes, core chamilo refactoring) |
|
Lint errors were found. A patch is also available. Please see the report: https://flintci.io/repositories/1013/analyses/37789 This comment was posted by FlintCI. It can be disabled in the repository settings. |
| * ) | ||
| * @ORM\JoinColumn(name="c_id", referencedColumnName="id") | ||
| */ | ||
| protected $course; |
There was a problem hiding this comment.
@jmontoyaa Can this association mapping for entities generate a change in the DB structure?
There was a problem hiding this comment.
Yes, It will create FKs in the database.
There was a problem hiding this comment.
Which should not create any problem, since only new Chamilo installations will get this new foreign key constraint (which is a security). Existing database will not be altered.
There was a problem hiding this comment.
...which leaves us with only the large changes to the core processes of Chamilo (even if only a change in form), which is a bit too risky to include in the code a few days before the release (we have to live with that with users support for months if there is something wrong).
The inclusion of this PR is thus postponed to after the 1.11.12 release.
I want to stress that this is not about bad code or anything (to the countrary). It is about the size of the changes and the corresponding risk.
learning path item removal does not cascade-remove its learning path AccessUrlRelCourse removal does not cascade-remove its course and url CItemProperty removal does not cascade-remove its session
…elCourse - refs BT#17507
|
Lint errors were found. A patch is also available. Please see the report: https://flintci.io/repositories/1013/analyses/39767 This comment was posted by FlintCI. It can be disabled in the repository settings. |
Moved most learning path creation code from main/lp/learnpath.class.php to CourseBundle/Entity/CLp.php and CLpItem.php.