fix: do not show hidden calendars

[SebastianKrupinski]

Resolves: #6767
Requires: nextcloud/server#51081

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.89%. Comparing base (2798772) to head (82316d5).
Report is 8 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6770      +/-   ##
============================================
+ Coverage     22.88%   22.89%   +0.01%     
- Complexity      476      478       +2     
============================================
  Files           252      252              
  Lines         12307    12309       +2     
  Branches       2383     2382       -1     
============================================
+ Hits           2816     2818       +2     
  Misses         9143     9143              
  Partials        348      348              

Flag	Coverage Δ
javascript	14.53% <ø> (ø)
php	59.37% <ø> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[SebastianKrupinski]

The failed tests are due to the missing class on server the required PR needs to be merged

[ChristophWurst]

This app works with Nextcloud 30. CI for 30 and 31 will still fail even when the server PR is in. Just FYI.

[SebastianKrupinski]

This app works with Nextcloud 30. CI for 30 and 31 will still fail even when the server PR is in. Just FYI.

Yes, the idea was to back port the server side to 30 and 31

[SebastianKrupinski]

@st3iny Do you mid giving this a final review?

[kesselb]

Pushed a commit to update the psalm-baseline:

• suppress isEnabled
• remove stale entries

Psalm has issues missing interfaces, does not handle interface_exists properly. Adding it to the baseline is therefore okay. The alternative is to safeguard the call with another method_exists check.

[kesselb]

Appended the previous commit to add psalm-baseline.xml to REUSE.toml. It's apparently not ideal to have the copyright in the baseline itself because that file is generated. Weird anyway to have a "copyright" for that, I mean it's generated after all 😕

[SebastianKrupinski]

Pushed a commit to update the psalm-baseline:

* suppress isEnabled

* remove stale entries

Psalm has issues missing interfaces, does not handle interface_exists properly. Adding it to the baseline is therefore okay. The alternative is to safeguard the call with another method_exists check.

I can change it to method_exists.

[kesselb]

Tested and works.

I believe it's still okay for me to approve this, as I only made changes related to CI and did not modify any code.

[kesselb]

I can change it to method_exists.

Sure! Feel free to drop my commit, I can take care of updating the baseline/moving spdx to resuse.toml later.

There is now more psalm errors, but in unrelated code 🤣

[kesselb]

There is now more psalm errors, but in unrelated code 🤣

That happend because I had nextcloud/ocp:dev-stable31 installed for updating the baseline and that dropped the entries for ServerVersion which is new in 31, but not available in 30.

composer require nextcloud/ocp:dev-stable30
composer run psalm -- --set-baseline=tests/psalm-baseline.xml
git restore composer.json composer.lock
composer install

[SebastianKrupinski]

There is now more psalm errors, but in unrelated code 🤣

That happend because I had nextcloud/ocp:dev-stable31 installed for updating the baseline and that dropped the entries for ServerVersion which is new in 31, but not available in 30.

composer require nextcloud/ocp:dev-stable30
composer run psalm -- --set-baseline=tests/psalm-baseline.xml
git restore composer.json composer.lock
composer install

Okay, I'll take care of the other errors

[SebastianKrupinski]

/backport to stable5.2

[kesselb]

This PR seems a bit cursed 🧙‍♂️ 😆

Tests need an update.

The baseline contains an entry that is no longer valid due to the method_exists check. Please drop my commit; I'll take care of updating the baseline and SPDX in a follow-up.

[SebastianKrupinski]

The baseline contains an entry that is no longer valid due to the method_exists check. Please drop my commit; I'll take care of updating the baseline and SPDX in a follow-up.

Will do

[SebastianKrupinski]

@st3iny when you get a min, remove your block please

[SebastianKrupinski]

@kesselb I think the easiest would be to back port the server portion to 30, its a fix for a issue

Test are failing on 30 because the interface is missing, unless they can be skipped for 30, but not sure that is possible since we need to create a dummy interface, for the inheritance issue

[st3iny]

As much as I dislike saying that, please remove the unit test again. Unfortunately, we can't mock the intersection of multiple interfaces until we upgrade to PHPUnit 10.

For now, test only with the old interface (ICalendar) and pretend that ICalendarIsEnabled does not exist.

Edit: See my next review.

[st3iny]

Hackity hack but should work ...

[SebastianKrupinski]

Hackity hack but should work ...

I agree, wacky hacky, the best option is just to back port the server code, its a fix for a bug and 30 is still supported. I'll back port it today, now that the freeze is over.

[SebastianKrupinski]

Hackity hack but should work ...

Looks like back porting to 30 was my original idea, the PR was already created was just waiting for the freeze to be over. 😆

[SebastianKrupinski]

/backport to stable5.3