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

System: Modifying entity with annotations for user_id. the previous one is commented #3464

Closed
wants to merge 21 commits into from

Conversation

carlangas159
Copy link
Contributor

All found user_ids have been changed to conform to annotated doctrine. The previous elements remain commented for security - refs #3463

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/39879

This comment was posted by FlintCI. It can be disabled in the repository settings.

src/CoreBundle/Entity/ClassUser.php Show resolved Hide resolved
src/CoreBundle/Entity/GradebookCertificate.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/GradebookEvaluation.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/GradebookLink.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/GradebookResult.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/GradebookResultLog.php Outdated Show resolved Hide resolved
Copy link
Member

@AngelFQC AngelFQC left a comment

Choose a reason for hiding this comment

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

The value for inversedBy in JoinColumn annotation is incorrect. Because it should be the class property in otherside entity.

src/CoreBundle/Entity/UserRelUser.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/UserRelTag.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/UserRelCourseVote.php Outdated Show resolved Hide resolved
@jmontoyaa
Copy link
Member

@carlangas159 Algun update de este PR?

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/41979

This comment was posted by FlintCI. It can be disabled in the repository settings.

@carlangas159
Copy link
Contributor Author

carlangas159 commented Sep 2, 2020

He colocado algunas relaciones en las entidades, corrijanme si me equivoco por favor

$admin -> 1 admin / 1 user

$gradebookCertificate -> n gradebookCertificate / 1 user
$gradebookEvaluation -> n gradebookEvaluation / 1 user
$gradebookLink -> n gradebookLink / 1 user
$gradebookResult -> n gradebookResult / 1 user
$gradebookResultLog -> n gradebookResultLog / 1 user
$gradebookScoreLog -> n gradebookScoreLog / 1 user
$sequenceValue -> n sequenceValue / 1 user
$trackEAttemp -> n trackEAttemp / 1 user
$trackECourseAccessRepository -> n trackECourseAccessRepository / 1 user
$userCourseCategory -> n userCourseCategory / n1 user
$userRelCourseVote -> n userRelCourseVote / 1 user
$userRelTag -> n userRelTag / 1 user
$userRelUser -> n userRelUser / 1 user

$templates -> n templates / 1 user - tengo dudas sobre este

He colocado la asociación de inversedBy en user respectivamente como lo especifica angel (muchisimas gracias), sin embargo, no estoy seguro si estas relaciones de 1 a muchos esten totalmente correctas.

Intente aplicar un patch de flinci pero se modifican muchos archivos y estructuras, por ahora lo dejo sin aplicar.

@jmontoyaa

* CHAMILO/master: (187 commits)
  Internal: Remove unused code, fix php warning
  Internal: Improve document list view
  Internal: Add js lib pretty-bytes
  Internal: Remove unused properties.
  Internal: Remove unused Cdocument properties
  Internal: Fix coding standards
  Set body padding to show breadcrumb
  Show footer
  Minor - format code
  Internal: Fix return resource list
  Internal: Fix table sorting and pagination
  Internal: Refactor view URL, improve show file page, fix js errors
  Internal: Remove unused social/download.php
  Internal: Minor - Add AccessDeniedHandler.php
  Customize sidebar
  Internal: Fix LINK crud
  Internal: Update yarn.lock
  Internal: Fix queries, use iid instead of id
  Internal: Add course list change button layout
  Internal: Add course list layout
  ...
@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/41981

This comment was posted by FlintCI. It can be disabled in the repository settings.

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/42059

This comment was posted by FlintCI. It can be disabled in the repository settings.

@carlangas159 carlangas159 changed the title WIP: System: Modifying entity with annotations for user_id. the previous one is commented System: Modifying entity with annotations for user_id. the previous one is commented Sep 4, 2020
@jmontoyaa
Copy link
Member

Aún no agregar api platform este PR es solo de las entidades de usuario.

src/CoreBundle/Entity/Admin.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/Admin.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/GradebookCertificate.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/GradebookCertificate.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/SequenceValue.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/TrackEAttempt.php Show resolved Hide resolved
src/CoreBundle/Entity/User.php Outdated Show resolved Hide resolved
src/CoreBundle/Entity/User.php Outdated Show resolved Hide resolved
* CHAMILO/master: (23 commits)
  Internal: Add "All" option in document list + add translations.
  Internal: Add psalm fixes
  Internal: Add justification plugin from 1.11.x
  Internal: Add files to psalm, fix psalm errors.
  Internal: Remove vuetify components, replace with bootstrap-vue
  Internal: Remove vuetify from vue.js
  Internal: Remove c_id, session_id from CAttendance.php
  Internal: Format code, remove unused code, add psalm check dirs
  Internal: Remove unused bundle
  Internal: Update forms with bootstrap v4
  Internal: Fix php fatal error
  Internal: Fix update error
  Internal: Add title when saving a new resource
  Internal: Fix folder creation via api
  Internal: Replace deprecated datetimepicker with flatpickr
  Internal: Add requiresAuth in vue pages
  Internal: Improve conditional nav in sidebar
  Internal: Sidebar show admin links to admin
  Internal: Use mapGetter to get store properties
  Internal: Fix add course when creating course
  ...

# Conflicts:
#	src/CoreBundle/Entity/User.php
…d functions. The following variables have been placed in the plural:

> $gradebookCertificates
> $gradebookEvaluations
> $gradebookLinks
> $gradebookResults
> $gradebookResultLogs
> $gradebookScoreLogs
> $sequenceValues
> $trackEExerciseConfirmations
> $templates
> $trackEAttemps
> $trackECourseAccess
> $userCourseCategorys
> $userRelCourseVotes
> $userRelTags
> $gradebookLinkevalLogs
> $userRelationships

  - refs chamilo#3464
@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/42401

This comment was posted by FlintCI. It can be disabled in the repository settings.

@@ -4,11 +4,25 @@

namespace Chamilo\CoreBundle\Entity;

use ApiPlatform\Core\Annotation\ApiFilter;
Copy link

Choose a reason for hiding this comment

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

Possibly zero references to use statement for classlike/namespace ApiFilter (\ApiPlatform\Core\Annotation\ApiFilter)

@@ -4,11 +4,25 @@

namespace Chamilo\CoreBundle\Entity;

use ApiPlatform\Core\Annotation\ApiFilter;
use ApiPlatform\Core\Annotation\ApiResource;
Copy link

Choose a reason for hiding this comment

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

Possibly zero references to use statement for classlike/namespace ApiResource (\ApiPlatform\Core\Annotation\ApiResource)

@@ -4,11 +4,25 @@

namespace Chamilo\CoreBundle\Entity;

use ApiPlatform\Core\Annotation\ApiFilter;
use ApiPlatform\Core\Annotation\ApiResource;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\BooleanFilter;
Copy link

Choose a reason for hiding this comment

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

Possibly zero references to use statement for classlike/namespace BooleanFilter (\ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\BooleanFilter)

use ApiPlatform\Core\Annotation\ApiFilter;
use ApiPlatform\Core\Annotation\ApiResource;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\BooleanFilter;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter;
Copy link

Choose a reason for hiding this comment

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

Possibly zero references to use statement for classlike/namespace SearchFilter (\ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter)

@@ -4,11 +4,25 @@

namespace Chamilo\CoreBundle\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
Copy link

Choose a reason for hiding this comment

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

Possibly zero references to use statement for classlike/namespace ApiResource (\ApiPlatform\Core\Annotation\ApiResource)

… collectionOperations and itemOperations to get. Removed Duplicate attributes={"security"="is_granted('ROLE_ADMIN')"} from apiResoruce. - refs chamilo#3464
@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/42442

This comment was posted by FlintCI. It can be disabled in the repository settings.

@carlangas159
Copy link
Contributor Author

Se han añadido los elementos de CourseBundle que tienen user_id.
Se ha ajustado los modelos para que api platform pueda cargarlos, por eso he añadido el siguiente bloque en el comentario de la clase en aquellas que no tenían dicha documentación

@ApiResource(
attributes={"security"="is_granted('ROLE_ADMIN')"},
iri="",
normalizationContext={"groups"={"user:read"}},
denormalizationContext={"groups"={"user:write"}},
collectionOperations={"get"},
itemOperations={"get"}
)

Para prevenir cualquier modificación actual por api, solo he dejado el metodo get asi se podria ir ajustando conforme se requiera.

También el las propiedades se ha añadido, según la documentación para apiplatform, el tipo y la propiedad, esto para que pueda ajustarse la documentación fácilmente.

@var User
@ApiProperty(iri="http://schema.org/Person")

También aplica para su contraparte en user.php

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5970 lines exceeds the maximum allowed for the inline comments feature.

@jmontoyaa
Copy link
Member

Como repito, favor de remover apiplatorm de este PR y efectuar los cambios solicitados solo para el tema de user id y doctrine.

No se ha solicitado implementar api platform todavía en todas la entidades.

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/42478

This comment was posted by FlintCI. It can be disabled in the repository settings.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5423 lines exceeds the maximum allowed for the inline comments feature.

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/42481

This comment was posted by FlintCI. It can be disabled in the repository settings.

@jmontoyaa
Copy link
Member

Corregir lo que indica flint https://flintci.io/repositories/1013/analyses/42481

Una vez que flint valide el código revisar lo que indica el action PHP Composer / PHP 7.3

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/42491

This comment was posted by FlintCI. It can be disabled in the repository settings.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5194 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5160 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5160 lines exceeds the maximum allowed for the inline comments feature.

@@ -4,6 +4,7 @@

namespace Chamilo\CoreBundle\Entity;

use ApiPlatform\Core\Annotation\ApiProperty;
Copy link
Member

Choose a reason for hiding this comment

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

Eliminar clase

Copy link
Member

Choose a reason for hiding this comment

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

Eliminar clase

@jmontoyaa
Copy link
Member

Sería una buena idea de usar traits en lugar de re escribir getUser y setUser, el trait puede ser creado aquí:

/srv/http/chamilo2/src/CoreBundle/Traits/UserTrait.php

Ya existe un trait para los cursos por si acaso:

/srv/http/chamilo2/src/CoreBundle/Traits/CourseTrait.php

@jmontoyaa
Copy link
Member

Revisar que las funciones eliminadas sean reemplazadas:

Ejemplo:

Antes:

$userId = $obj->getUserId();

Después:

$userId = $obj->getUser()->getId();

* CHAMILO/master: (120 commits)
  Internal - Add behat tests, fix sql errors
  Internal - Add course registration behat test, fix php errors
  Internal - Minor - fix behat test
  Internal - Fix queries, fix installation, format code.
  Internal - Fix installation
  Internal - Add install logs
  Internal - Format code
  Internal - Fix behat tests
  Internal - Fix behat test
  Internal - Debug installation, remove unused field, fix deprecations.
  Internal - Improve global.inc.php loading (disable debug) +fix error tpl
  Internal - Fix behat tests
  Internal - Fix lang vars, fix course public URL
  Internal - Add voters when using api platform, fix documents visibility.
  Internal - Fix behat tests, fix lang vars
  Internal - Add permission to DRH to login as other user
  Internal - Fix behat tests, queries
  Internal - Fix php errors, use course.id instead of course.code
  Internal - Minor - tpl format code
  Internal - Fix group behat tests
  ...

# Conflicts:
#	src/CoreBundle/Entity/User.php
@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/44755

This comment was posted by FlintCI. It can be disabled in the repository settings.

@carlangas159
Copy link
Contributor Author

Se han cambiado los setUser y getUser por la implementación de trait. sin embargo, no he podido evaluar aún setUser y getUser en código legacy, ya que debo generar el escenario para cada parte y poder probarlo

image

El cual afectaría para el momento de su ejecución.

image

He tratado de generar un escenario para quiz, añadiendo una pregunta a un ejercicio en un curso y me salio este aviso
image

Por lo momento, la verificación de remover setUserId para setUser tomará tiempo para poder evaluar cada elemento en legacy

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5187 lines exceeds the maximum allowed for the inline comments feature.

* ChamiloMaster:
  Internal - Fix legacy code, use resources
  Internal - Fix queries, fix behat tests
  Internal - Fix queries, fix behat tests
  Internal - Fix LP creation
  Internal - Fix entities
  Internal - Fix fill courses
  Internal - Fix delete course
  Internal - Add behat debug
  Internal - Fix add course with content
  Internal - Fix behat test link
  Internal - Fix queries, add lp behat test
  Internal - Add more behat tests, fix php errors.
  Internal - Add admin icons
  Internal - Add/Fix behat test
@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/44794

This comment was posted by FlintCI. It can be disabled in the repository settings.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5253 lines exceeds the maximum allowed for the inline comments feature.

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/44795

This comment was posted by FlintCI. It can be disabled in the repository settings.

@FlintCIBot
Copy link

Lint errors were found. A patch is also available.

Please see the report: https://flintci.io/repositories/1013/analyses/44800

This comment was posted by FlintCI. It can be disabled in the repository settings.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5266 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Sep 24, 2020

Code Climate has analyzed commit 0a0f4dd and detected 134 issues on this pull request.

Here's the issue category breakdown:

Category Count
Bug Risk 127
Style 7

Note: there are 12 critical issues.

View more on Code Climate.

public/main/exercise/exercise.class.php Show resolved Hide resolved
public/main/forum/forumfunction.inc.php Show resolved Hide resolved
*/
protected $pictureUri;
protected $userCourseCategorys;
Copy link
Member

Choose a reason for hiding this comment

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

protected $userCourseCategories

Copy link
Contributor Author

Choose a reason for hiding this comment

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

en src/CoreBundle/Entity/TrackEAttempt.php no se podría eliminar ya que depende por cursos

/**
* @var Course
* @ApiProperty(iri="http://schema.org/Course") <<<<
* @Orm\ManyToOne(
* targetEntity="Chamilo\CoreBundle\Entity\Course",
* inversedBy="trackEAttempts"
* )

src/CoreBundle/Entity/User.php Show resolved Hide resolved
src/CoreBundle/Entity/User.php Show resolved Hide resolved
src/CoreBundle/Entity/User.php Show resolved Hide resolved
src/CoreBundle/Entity/User.php Show resolved Hide resolved
@jmontoyaa
Copy link
Member

Solamente cambiar las llamadas $userId = $obj->getUserId(); por lo nuevo, los ejercicios aún no funcionan en v2 y hay muchas cosas por corregir aún.

@jmontoyaa
Copy link
Member

Voy ha agregar estos cambios parcialmente a master

@jmontoyaa
Copy link
Member

He agregado los cambios hechos en src/CoreBundle.
Los cambios hechos a src/CourseBundle no serán pasados, pues esas entidades van ha hacer resources y los resources tienen datos del autor en resourceNode.creator

@jmontoyaa jmontoyaa closed this Sep 30, 2020
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.

None yet

4 participants