OHRM5X-2484: Develop I18NLanguage delete API #1812
Conversation
There was a problem hiding this comment.
Great job! 🤩 Now you need to add the tests and make changes to the data groups.
Testing References:
- https://github.com/orangehrm/orangehrm/blob/main/src/plugins/orangehrmPerformancePlugin/test/Api/KpiAPITest.php
- https://github.com/orangehrm/orangehrm/blob/main/src/plugins/orangehrmAdminPlugin/test/Dao/JobTitleDaoTest.php
Data group update needs to be done in ohrm_data_group and ohrm_user_role_data_group
|
Also note that we put the ticket ID in the PR title. You can edit the title and add it Example: #1144 |
|
Congrats on the first PR 🎊 |
|
Thanks guys. Still not complete.
Thanks Devishke for the guidance.
…On Fri, Feb 16, 2024, 6:37 PM Chamara Abesinghe ***@***.***> wrote:
Congrats on the first PR 🎊
—
Reply to this email directly, view it on GitHub
<#1812 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMP2ZAHAQLBII5V5VTPVMMDYT5K2TAVCNFSM6AAAAABDLYKW6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBYGM2TQMJRGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
src/plugins/orangehrmAdminPlugin/test/Api/I18NLanguageAPITest.php
Outdated
Show resolved
Hide resolved
| 'with deleted language id': | ||
| userId: 1 | ||
| services: | ||
| admin.localization_service: OrangeHRM\Admin\Service\LocalizationService | ||
| attributes: | ||
| id: 7 | ||
| exception: | ||
| class: \OrangeHRM\Core\Api\V2\Exception\RecordNotFoundException | ||
| message: 'Record Not Found' |
There was a problem hiding this comment.
this test is the same as the previous since only 5 language records are loaded into test DB from the fixtures
we need to add a new parameter to getOne endpoint like getAll's activeOnly
https://github.com/orangehrm/orangehrm/blob/main/src/plugins/orangehrmAdminPlugin/Api/I18NLanguageAPI.php#L52-L57
src/plugins/orangehrmAdminPlugin/test/fixtures/testcases/I18NLanguagesAPITestCase.yml
Outdated
Show resolved
Hide resolved
| public const REMOVED = 0; | ||
| public const ADDED = 1; |
There was a problem hiding this comment.
nice. can we refactor any other places that uses these values
| @@ -0,0 +1,44 @@ | |||
| <?php | |||
| /** | |||
There was a problem hiding this comment.
@devishke-orange any possibility to check the copyright header in lint workflow. I think there should be a way. https://cs.symfony.com/doc/rules/comment/header_comment.html
TharakaAthukorala
changed the title
…tead of "enabled" column in ohrm_i18n_language table.
…ts and db migrations after review
TharakaAthukorala
force-pushed
the
5.7
branch
from
24cf545
to
3a2fcef
Compare
| @@ -0,0 +1,44 @@ | |||
| <?php | |||
| /** | |||
There was a problem hiding this comment.
@devishke-orange any possibility to check the copyright header in lint workflow. I think there should be a way. https://cs.symfony.com/doc/rules/comment/header_comment.html
Develop I18NLanguage delete API without unit testing & db migrations for the admin user role, and data group permissions.