Additional tests and fixes #35

scottmcdonnell · 2024-02-25T15:33:43Z

I had a few issues with the 1.1.0 sync.

tests were not passing 100% for me
user names with spaces failing validation
users with no last name failing validation
concept task type missing - gazu.task.all_task_types_for_project(kitsu_project_id) => gazu.task.all_task_types()
concept where data =None was causing calculate_end_frame to fail
added fixtures to update the studio_settings for tests
added tests for entity Person
added tests for entity Edit
added tests for entity Concept
user isAdmin and isManager were not being set for me
added the the accessGroup to the project as users were not being assigned without it
delete projects setting bug

Lastly some of new bits were using the Ayon api via httpx to save users/projects etc but this is mixed with the existing server/kitsu/utils.py which uses Postgres for gets and Entity models to save and update. This I found really confusing, it should be all one or the other. It didn't make much sense to me for Ayon (in an ayon api endpoint) to be calling itself through the api when the models are available and the postgres connection is already initialised, it seems an unnecessary overhead, so I converted the api calls to model calls in utils.py to make it consistent.

I saw afterwards that this was discussed on discord a bit, and the preference was API, but I think it should be consistent throughout utils.py - so not sure what the team thinks on this one!

…ng_sync

…passing

…ngs.delete_projects - tests for deleting project with setting enabled and disabled

…we skip onto the next entity rather than stopping.

…ess groups

…ils into its own module for easy testing.

scottmcdonnell · 2024-03-14T21:17:29Z

added support for LauncherAction #36

(cherry picked from commit b1da619020ec2b4d5ed1b2d6b36d4303bae052c4)

EmberLightVFX · 2024-03-27T17:50:48Z

Would it be possible to change the arrow-icon to the Kitsu icon for the "Show in Kitsu" button?
You can get their icon here: https://www.cg-wire.com/_nuxt/logo-kitsu.de716c4b.png (might want to make it smaller)

iLLiCiTiT · 2024-04-22T09:54:37Z

Hello, could you update the changes? Could you also split the PR into more PRs? There is too much changes for review (smaller PRs -> faster review).

scottmcdonnell · 2024-05-03T09:13:38Z

Hello, could you update the changes? Could you also split the PR into more PRs? There is too much changes for review (smaller PRs -> faster review).

Thanks for taking a look. I can give it a go breaking it down into different PRs.
Before I go ahead can I ask you to review one big structural change in this PR to see if I should proceed down this path?

on the server side there is a mix between using EntityModels and api calls in the current code. This PR replaces any api with Entity in order to make it consistent. From above:

Lastly some of new bits were using the Ayon api via httpx to save users/projects etc but this is mixed with the existing server/kitsu/utils.py which uses Postgres for gets and Entity models to save and update. This I found really confusing, it should be all one or the other. It didn't make much sense to me for Ayon (in an ayon api endpoint) to be calling itself through the api when the models are available and the postgres connection is already initialised, it seems an unnecessary overhead, so I converted the api calls to model calls in utils.py to make it consistent.

For example:
7559d8b

iLLiCiTiT · 2024-05-03T10:08:07Z

Before I go ahead can I ask you to review one big structural change in this PR to see if I should proceed down this path?

The issue is that different changes should be reviewed by different people, that's why I asked for split. It is hard to tell which change is related to what.

There are some small bugfixes and big feature changes next to it (multiple features). The features usually needs discussion or may not be merged because of different opinion, but the bugfixes can be merged right away.

scottmcdonnell · 2024-05-03T11:04:15Z

Before I go ahead can I ask you to review one big structural change in this PR to see if I should proceed down this path?

The issue is that different changes should be reviewed by different people, that's why I asked for split. It is hard to tell which change is related to what.

There are some small bugfixes and big feature changes next to it (multiple features). The features usually needs discussion or may not be merged because of different opinion, but the bugfixes can be merged right away.

Fair enough, I get that. Would it be possible to get feedback on the replacement of API for Entities please?
7559d8b

If that is given the thumbs up or down I will know how to proceed with the PRs.

iLLiCiTiT · 2024-05-03T11:14:07Z

Fair enough, I get that. Would it be possible to get feedback on the replacement of API for Entities please?
7559d8b

@martastain

Note that it is hard to tell, because for example I don't know which exact changes are related to it.

m-u-r-p-h-y · 2024-05-06T15:55:42Z

First of all thank you @scottmcdonnell for your contribution! To push it through, we really need to split this massive PR to smaller pieces as it is not approvable in the current state.

each bulleted item in your description should be a separate PR honestly.

scottmcdonnell · 2024-05-20T15:12:58Z

First of all thank you @scottmcdonnell for your contribution! To push it through, we really need to split this massive PR to smaller pieces as it is not approvable in the current state.

each bulleted item in your description should be a separate PR honestly.

No problem, I will break into smaller PRs. I am on a few big projects at the moment but will come back to this as soon as I can.

BigRoy · 2024-06-03T20:21:04Z

Would be great to get some 🔥 progress into this one.
Someone else just hit what seems like something that this PR may solve? See on Ynput discord here.

scottmcdonnell · 2024-06-05T15:38:22Z

Would be great to get some 🔥 progress into this one. Someone else just hit what seems like something that this PR may solve? See on Ynput discord here.

Sorry was very snowed under. PRs started. Can I make PRs dependant on each other? The testing requires fixtures and set up that is common to each PR and some changes have larger dependencies.

MustafaJafar · 2024-07-01T11:59:40Z

Hello,
I think this PR is big, would be better to break it down into smaller PRs ?
e.g. could we have separate PR for the launcher action ?

scott added 26 commits February 20, 2024 18:32

improve test readme

a658564

fixtures for users and studio_settings, initial test for kitsu Person

563e94b

return users from push for testing

027140e

bugfix - handle kitsu person name with spaces or special characters

0a19422

Merge remote-tracking branch 'ayon/main' into enhancement/fully_worki…

bea877f

…ng_sync

do not set password if blank

4d7844d

UserEntity instead of httpx client call for user update.

7559d8b

fix mock data and tests for person entities

49c2e61

handle change of user name

95d9dad

bugfix - only calculate endframe if not explicitly set.

cfb8b4b

ProjectEntity instead of httpx, project anatomy updating - all tests …

43277b8

…passing

bugfix - settings.delete_ayon_projects.enabled => settings.sync_setti…

cf3964f

…ngs.delete_projects - tests for deleting project with setting enabled and disabled

optional param for mock and test improvements

c8c87a4

fix for usernames with no first or last name failing ayon validation

8bac56e

try catch for entities push so if they fail the entity is logged and …

3ce2b48

…we skip onto the next entity rather than stopping.

test cleanup and version bump

6fc25dd

bugfix for isAdmin and isManager to set correctly. add project to acc…

3d2653c

…ess groups

try and catch for remove entity so problems can be logged and skipped

5f5e996

bug fix for concepts having no data

2e90aa9

tests for edits and concepts

bcd1bcb

bugfix for concept task type missing as not on the project

abdb91b

init_pairing test fix

61c2bc1

bugfix support names with unicode characters - move any formatting ut…

caaf7e6

…ils into its own module for easy testing.

do not sync Kitsu bots

d3b3639

remove ayon_server_url as unnecessary

12d4cbe

support for LauncherAction - Show In Kitsu

e5c423b

scottmcdonnell mentioned this pull request Mar 14, 2024

Show In Kitsu - LauncherAction #36

Open

scott added 2 commits March 14, 2024 22:01

fix all tests to support kitsuType on folders

b28825a

working launcher_show_in_kitsu

058de90

(cherry picked from commit b1da619020ec2b4d5ed1b2d6b36d4303bae052c4)

add version for direct mounting ayon-kitsu/server in ayon-docker

7f3e189

scottmcdonnell mentioned this pull request Mar 15, 2024

Enhancement/comment template #39

Merged

MustafaJafar requested a review from iLLiCiTiT April 19, 2024 12:20

MustafaJafar linked an issue Apr 19, 2024 that may be closed by this pull request

Show In Kitsu - LauncherAction #36

Open

m-u-r-p-h-y assigned scottmcdonnell May 7, 2024

scottmcdonnell mentioned this pull request Jun 13, 2024

Enhancement/project entity #57

Merged

MustafaJafar added type: bug Something isn't working community Issues and PRs coming from the community members test labels Jul 1, 2024

iLLiCiTiT added tests PR contains new unit or integration test or improves the existing ones and removed test labels Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional tests and fixes #35

Additional tests and fixes #35

scottmcdonnell commented Feb 25, 2024

scottmcdonnell commented Mar 14, 2024

EmberLightVFX commented Mar 27, 2024 •

edited

Loading

iLLiCiTiT commented Apr 22, 2024

scottmcdonnell commented May 3, 2024

iLLiCiTiT commented May 3, 2024 •

edited

Loading

scottmcdonnell commented May 3, 2024

iLLiCiTiT commented May 3, 2024 •

edited

Loading

m-u-r-p-h-y commented May 6, 2024

scottmcdonnell commented May 20, 2024

BigRoy commented Jun 3, 2024

scottmcdonnell commented Jun 5, 2024

MustafaJafar commented Jul 1, 2024

Additional tests and fixes #35

Are you sure you want to change the base?

Additional tests and fixes #35

Conversation

scottmcdonnell commented Feb 25, 2024

scottmcdonnell commented Mar 14, 2024

EmberLightVFX commented Mar 27, 2024 • edited Loading

iLLiCiTiT commented Apr 22, 2024

scottmcdonnell commented May 3, 2024

iLLiCiTiT commented May 3, 2024 • edited Loading

scottmcdonnell commented May 3, 2024

iLLiCiTiT commented May 3, 2024 • edited Loading

m-u-r-p-h-y commented May 6, 2024

scottmcdonnell commented May 20, 2024

BigRoy commented Jun 3, 2024

scottmcdonnell commented Jun 5, 2024

MustafaJafar commented Jul 1, 2024

EmberLightVFX commented Mar 27, 2024 •

edited

Loading

iLLiCiTiT commented May 3, 2024 •

edited

Loading

iLLiCiTiT commented May 3, 2024 •

edited

Loading