- refactor with_test_app so it allows to run tests without automatically starting the test app
- add reverse_single_reference field type - Ref. T763
Details
- Reviewers
bartosz-gordon piotr-ptaszynski - Group Reviewers
Unknown Object (Project) - Maniphest Tasks
- T763: Field type reverse single reference
- Commits
- rSdbcb52182b2b: Reverse single reference
rS1be208d571ce: Reverse single reference
Diff Detail
- Repository
- rS Sealious
- Branch
- reverse-reference (branched from alpha)
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 486 Build 486: arc lint + arc unit
Event Timeline
lib/app/app.js | ||
---|---|---|
106 | Czy czystszym podjeściem nie będzie modyfikowanie tej flagi z poziomu dedykowanej funkcji? |
lib/app/app.js | ||
---|---|---|
106 | Co zyskalibyśmy w ten sposób? |
lib/app/app.js | ||
---|---|---|
106 | Czytałem o tym w książce "Czysty kod" ale nie pamiętam uzasadnienia, jak znajdę czas to poszukam |
lib/app/app.js | ||
---|---|---|
106 | Ok, jak poznam zalety takiego rozwiązania w tym przypadku to z chęcią wdrożę zmiany ;) Jeżeli znajdziesz to uzasadnienie po wylądowaniu tego diffa, to zrób proszę komentarz w audycie do powiązanego z nim commita |
Failuje poniższy test - przy imporcie with-test-app zapomniałeś zdestrukturyzować { with_running_app }.
lib/app/base-chips/field-types/reverse-single-reference.js | ||
---|---|---|
25 | (question) Jest jakiś powód, dla którego używasz tutaj params.field_name, a poniżej samego field_name? | |
42 | (should) Na wszelki wypadek można tu zmienić == na === | |
80 | (question) Czemu służy ta linia? :P | |
lib/app/metadata.js | ||
6 | (question) Czy czasem nie chciałeś użyć tutaj zmiennej COLLECTION_NAME, a nie stringa? :P |
Idzie request changes, ale kawał dobrej roboty!
lib/app/app.js | ||
---|---|---|
106 | Wtrącę swoje 3 grosze, że rzeczywiście to co mówi Arkadiusz, to jest dobra praktyka - uniezależniamy się od reprezentacji stanu, natomiast w tym wypadku, jak to jest 7 przypisań w 2 plikach i jest to fajnie przesłonięte w testach, to na razie bym nie zmieniał. Jak się zacznie rozrastać, to rzeczywiście warto | |
lib/app/base-chips/collections/password-reset-intents.subtest.js | ||
20–21 | (question) Wiem, że to przyszło w ramach innego taska, ale czy na pewno to jest dobra praktyka? Czy aplikacja powinna zdradzać, że dane konto nie istnieje? | |
lib/app/base-chips/field-types/reverse-single-reference.js | ||
72 | (should) | |
105 | (could) Znowu jest miejsce na redukcję nestingu - jeżeli format_params[0] nie przyjmuje żadnej z wartości oraz może, dla spójności, zamiast return undefined zrobić return decoded_value. | |
111 | (could) | |
lib/app/base-chips/field-types/reverse-single-reference.subtest.js | ||
42 | (could) A może const bs = await Promise.map(...) i wyleci for? | |
66 | Nieznałem tego - fajne! | |
88 | (should) Ten fragment: await with_stopped_app(async ({ app, rest_api }) => { await create_referencing_collections(app, "with_reverse" && true); await app.start(); await create_resources(app);}) powtarza się w każdym teście - moim zdaniem można to łatwo wyłączyć do osobnej funkcji przyjmującej parametr with_reverse | |
179 | (question) Dlaczego 3? | |
test_utils/with-test-app.js | ||
87 | (could) Skoro się powtarza w try i catch to dobry przypadek użycia sekcji finally |
lib/app/base-chips/collections/password-reset-intents.subtest.js | ||
---|---|---|
20–21 | Tak, bo aplikacja i tak by zdradzała ten fakt pozwalając lub nie założyć konto na dany adres ;) | |
lib/app/base-chips/field-types/reverse-single-reference.js | ||
25 | Tak. field_name to nazwa pola, do którego jest przypisany nasz reverse-single-reference. params.field_name to nazwa pola typu single-reference, które jest w jakiejś osobnej kolekcji. Skoro nie jest to oczywiste, najwyraźniej niefortunnie dobrałem nazwy zmiennych. Pomyślę, jak by je zmienić. | |
80 | Ups, cały dzień kodzenia padł mi na mózg :P Już usuwam | |
lib/app/metadata.js | ||
6 | ups :D | |
test_utils/with-test-app.js | ||
87 | O, nie wiedziałem, że js to ma :x |
Jeżeli moje pytanie wynika z niezrozumienia to zachęcam do lądowania. A jeżeli nie, to zachęcam do poprawienia ;)
Najmocniej przepraszam, starałem się odpowiedzieć na każde z pytań, ale jakimś cudem ten jeden komentarz mi uleciał. Rozumiem, że chodzi o ten:
https://hub.sealcode.org/D200#inline-1343
odnosząc się do fragmentu:
results = await rest_api.get( "/api/v1/collections/B?filter[references_in_a][pairity]=odd" ); assert.equal(results.length, 3);
Jeżeli to jest fragment o który Ci chodzi, to odpowiedź jest następująca:
w setupie testu tworzymy 6 zasobów w A - połowę z polem "pairity" na "odd", a połowę na "even". Każdy z nich wskazuje na jeden z 6ciu elementów kolekcji B. Dlatego jest 6/2 = 3 zasobów w B na które wskazuje jakiś A z "pairity" ustawione na "odd"