[XI-6523] Transfer enmeshed changes from Xikolo #1831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1831 +/- ##
==========================================
+ Coverage 94.84% 94.87% +0.02%
==========================================
Files 133 133
Lines 3375 3393 +18
==========================================
+ Hits 3201 3219 +18
Misses 174 174 ☔ View full report in Codecov by Sentry. |
nenock
force-pushed
the
nn/XI-6523_transfer_enmeshed_changes_from_Xikolo
branch
6 times, most recently
from
4f38640 to
c5a2837
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
nenock
force-pushed
the
nn/XI-6523_transfer_enmeshed_changes_from_Xikolo
branch
from
c5a2837 to
3817303
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
nenock
force-pushed
the
nn/XI-6523_transfer_enmeshed_changes_from_Xikolo
branch
2 times, most recently
from
16ae249 to
5dad0f5
Compare
nenock
force-pushed
the
nn/XI-6523_transfer_enmeshed_changes_from_Xikolo
branch
from
ec816b1 to
7fe70a9
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
nenock
force-pushed
the
nn/XI-6523_transfer_enmeshed_changes_from_Xikolo
branch
3 times, most recently
from
18755f2 to
674adc9
Compare
MrSerth
reviewed
Feb 12, 2025
MrSerth
reviewed
Feb 12, 2025
- Increase code robustness Use `setTimeout` instead of `setInterval` to prevent queued up XHR requests returning unordered in case of e.g. an unresponsive server Part of XI-6523
…ibly `RelationshipTemplate#remaining_validity` requires the attribute `expire_at` to be of type `ActiveSupport::TimeWithZone` to be able to perform `minus_without_duration`. Part of XI-6523
- Group class methods with `class << self` - Write out abbreviations - Enmeshed::Connector: Combine initialization and memoization of the Faraday connection into a single method (no expensive calculations here) Part of XI-6523
…eouts Memorizing the connection without using Faraday's adapter `net_http_persistent` does not come with any benefits (every request requires setting up a new TCP socket with the default adapter). Part of XI-6523
Since RSpec 3.5, controller specs are deprecated. The official recommendation of the Rails team and the RSpec core team is to write request specs instead. They involve the router, the middleware stack, and both rack requests and responses. Thus, it's not possible to set the session variables beforehand anymore. Instead, a request spec should call the sign in endpoint before calling the actual endpoint under test, when the session is needed. To avoid the complexity of SSO and SLOs during request tests, this helper introduces the option to set the session variables via a designated endpoint for tests. https://gist.github.com/dteoh/99721c0321ccd18286894a962b5ce584?permalink_comment_id=4188995#gistcomment-4188995
Part of XI-6523
…t/*` Usually, when the current user is not logged in and unauthorized, a redirect to the registration page makes sense. But in case of the `NBPWalletController` actions, the SAML provider and uid are missing and the provided alert message is more meaningful. Part of XI-6523
Drop corresponding controller spec while maintaining full test coverage Part of XI-6523
Main changes:
- Relationship: Adapt parsing of the userdata to new schema
Values to the requested attributes are now passed in `creationContent/response/items`
(former: `changes/request/response/items`).
- Connector: Adapt accepting/rejecting a Relationship to new endpoints
`/api/v2/Relationships/#{relationship_id}/Changes/#{change_id}/#{action}` was dropped
in favor of `/api/v2/Relationships/#{relationship_id}/Accept` (and `Reject`)
Part of XI-6523
This decreases the Assignment Branch Condition size of the method.
Part of XI-6523
When appending a return statement to a method call, either with `and` or `&&`, the return won't happen if there is a tailing one-line if statement. Part of XI-6523
…s to `Attribute::Identity` The former implementation of `Attibute.to_json` was very specific to IdentityAttributes. It actually only worked for simple IdentityAttributes and would need adaptations for complex IdentityAttributes. Part of XI-6523
…` model Part of XI-6523
nenock
force-pushed
the
nn/XI-6523_transfer_enmeshed_changes_from_Xikolo
branch
from
674adc9 to
bd5a59a
Compare
Previously, the session was kept intact including the SAML information. This allowed users to return to the connect page, since they were still partially signed in. However, no visual indicator was shown for that state. We decided not to initiate the Single Log-Out via SAML or a regular logout due to the potentially increased complexity and interferences with the NBP IdP.
Previously, existing users signed up through NBP were allowed to view the connect page. With this change, only authenticated users not having an account yet are allowed to access the page.
This change is necessary to get proper error messages in case of (unhandled) exceptions. Previously, a `Pundit::NotAuthorizedError` would redirect to an HTML page. Now, such an exception with simply return a JSON with the proper status code of 401. Since we are using `fetch` with a path not containing any specific format, the previously used default would log `syntax error` messages in the JavaScript console (due to the followed redirect and the attempt to parse the then-received HTML as JSON).
MrSerth
approved these changes
Feb 13, 2025
There was a problem hiding this comment.
Awesome, thank you so much! I am happy to have all Xikolo-made changes integrated and adapted to CodeHarbor.
Through another round of extensive testing, I was able to confirm that everything enmeshed-related is working fine and as expected. 🎉 For my tests, I even pulled the latest version of the container and tested it in conjunction with the latest mobile app. 📱
During said testing, I discovered three further edge cases, which I fixed immediately (and pushed on your branch to speed up everything; I hope that's okay 😇). If you would find the time to quickly check my changes, I would be glad, but it's also fine if you won't. There are further details given in each of the commit messages.
With all of these changes and the additional time we spent, I am very confident about having developed a nice and clean solution. Many thanks, once again, to your dedication and the effort you've spent. It wasn't just an extra mile, it was more than that 🤗
Feel free to take the honor merging your PR; I am very proud of the result you've worked hard for. 🏆🏁
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR transfers and adapts the changes done in Xikolo, see https://lab.xikolo.de/xikolo/web/-/merge_requests/4665 .
Changes include: