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

Add basic filter test for layout resource #150

Merged

Conversation

bofalke
Copy link
Contributor

@bofalke bofalke commented Oct 11, 2022

Ref #113

First Test for the Layout Resource filter.

@bofalke bofalke changed the title Add basic filter test for layout resource Draft: Add basic filter test for layout resource Oct 11, 2022
@bofalke
Copy link
Contributor Author

bofalke commented Oct 11, 2022

Is this going in the right direction? I guess all i am missing now for the layout component are the different scenarios

  • Filtering by uiComponent
  • Order by createdAt
  • Order by Reference

@silverbackdan
Copy link
Collaborator

This looks good thanks.
May I suggest we have 2 scenarios?
1 which will test the layouts endpoint and another which checks that it can be filtered instead of merging both tests into 1?
Would you still like to have some Docker configs?

@bofalke
Copy link
Contributor Author

bofalke commented Oct 12, 2022

Yes i can split it into 2 scenarios 👍

The docker config would be helpful for manual playing around but i think i can add the test cases mentioned above without. So no need to rush it.

@silverbackdan
Copy link
Collaborator

Sorry for the delay, I'll be working on the docker configs now, see what I can get going.
If you need to run this repo you could always run the template repository, and map this bundle to your local machine in the composer.json file in the api directory - this way you'll be running everything - full stack with mercure, caddy, varnish etc.

@gitguardian
Copy link

gitguardian bot commented Oct 26, 2022

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@silverbackdan
Copy link
Collaborator

Hi @bofalke - sorry I've been extremely tied up with Antoine and many mercure updates and other fixes which I'm continuing to work on today actually - I think I've just finished my last update to main branch. Hopefully it won't require a rebase but if so, you should be fine to do this any time from now I hope as I'm moving onto front-end work.

@bofalke bofalke changed the title Draft: Add basic filter test for layout resource Add basic filter test for layout resource Oct 26, 2022
@bofalke
Copy link
Contributor Author

bofalke commented Oct 26, 2022

Sorry for the delay, i was a little busy the last weeks. However i think i finally managed to include all the tests for the layout component. The GitGuardian Security Check occured after i merged the main branch, i guess i will try and rebase it

@bofalke bofalke force-pushed the feature/#113-additional-tests branch 2 times, most recently from 9f939fd to 37d9f91 Compare October 26, 2022 12:50
@silverbackdan
Copy link
Collaborator

The gitguardian is OK, it's a dummy secret but looks real so I think I just need to make it look like a dummy :)

@silverbackdan
Copy link
Collaborator

These are very good tests thank you for your work on it. I'm happy to merge when you are finished and I can easily duplicate these tests for the page resource as well with these layout resource tests in place.
LMK when you're finished your side and I'll merge.

@silverbackdan
Copy link
Collaborator

Also lowest dependencies tests will fail for now until APIP core 3.0.3 is released. I didn't want the fixme statement in there and we will require that as the minimum dependency.

@bofalke
Copy link
Contributor Author

bofalke commented Oct 26, 2022

Okay it is green now as i removed the merge commit by rebasing onto the main branch. However Scrutinizer fails during composer install, anything i need to do here?

@bofalke bofalke force-pushed the feature/#113-additional-tests branch from 37d9f91 to 3a34540 Compare October 26, 2022 14:01
@codeclimate
Copy link

codeclimate bot commented Oct 26, 2022

Code Climate has analyzed commit 3a34540 and detected 0 issues on this pull request.

View more on Code Climate.

@silverbackdan
Copy link
Collaborator

Nah that's all good, thanks - merging now.
Thanks very much for the PR - really appreciated. I'll try remember the tool I used to list contributors and get you into the readme soon.

@silverbackdan silverbackdan merged commit 7db786e into components-web-app:main Oct 26, 2022
@silverbackdan silverbackdan mentioned this pull request Oct 26, 2022
5 tasks
@bofalke
Copy link
Contributor Author

bofalke commented Oct 26, 2022

Cool thanks!

I try to add some tests for the other components as well

@silverbackdan
Copy link
Collaborator

Amazing, thank you!

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.

2 participants