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

fix: Bug archived read only mode variants are editable even tho not persisted #257

Conversation

aiAdrian
Copy link
Collaborator

@aiAdrian aiAdrian commented Aug 20, 2024

Description

When the project is archived, the user can no longer modify the netzgrafik. All editing functions should be disabled.

Testing

This bug fix is a big one. Therefore, we have to deeply test the user interface interactions. There should be no differences from the current version as long as the project is not archived. If a project along with their variants are archived, the display of these variants is read-only. That means the user can no longer make any changes.

  • All manual tests passed

Checklist

  • This PR contains a description of the changes I'm making
  • I've read the Contribution Guidelines
  • I've added tests for changes or features I've introduced
  • I documented any high-level concepts I'm introducing in documentation/
  • CI is currently green and this is ready for review

@louisgreiner
Copy link
Collaborator

I will take a close look to this PR. Have you seen this related issue ? #191
The issue is that NGE integrated version currently runs with the bugged read-only mode (so yet not fixed by this PR). By merging this PR, the integrated versions of NGE will be broken. Had you have the time to think about a 3rd mode yet ?

@aiAdrian
Copy link
Collaborator Author

aiAdrian commented Aug 20, 2024

I will take a close look to this PR. Have you seen this related issue ? #191 The issue is that NGE integrated version currently runs with the bugged read-only mode (so yet not fixed by this PR). By merging this PR, the integrated versions of NGE will be broken. Had you have the time to think about a 3rd mode yet ?

Thank you for your comment. I believe integrating the standalone application will be quite simple. If environment.backendDisabled is set to true, the getVariantIsWritable method in VersionControlService should always return true. Additionally, the environment object could be extended with a flag - readonly: boolean. If readonly is set to true, everything can be handled in VersionControlService to enforce the read-only mode. Possibly, this method should be moved to another service, which requires a software architectural review that has not yet been done in detail. However, this can be easily achieved through refactoring.

Important method: VersionControlService.getVariantIsWritable()

@louisgreiner
Copy link
Collaborator

image

It would be nice to hide this grey bar on bottom on button where the mouse is (hover) ? Disable this bar when a button is disable

@aiAdrian
Copy link
Collaborator Author

I have overworked the styling and the behavior (only styling) when:

  • readonly
  • if Streckengrafik is on
  • if Analytics is on
    for the menu. It should now be finished/polished. But please re-test. Thanks.

@louisgreiner
Copy link
Collaborator

Shouldn't archived projects automatically archive variants that aren't archived yet ?

image

Here, the variant isn't archived, even tho the project is, but the if we open the variant, the mode is archived

image

The solution would be to automatically archive all the variants of a project that is archived, and tick and disable by default the Show archive button ?

image

@aiAdrian aiAdrian requested a review from SergeCroise August 27, 2024 05:57
@aiAdrian
Copy link
Collaborator Author

aiAdrian commented Aug 27, 2024

Shouldn't archived projects automatically archive variants that aren't archived yet ?

...

The solution would be to automatically archive all the variants of a project that is archived, and tick and disable by default the Show archive button ?

I'm not sure if I understood everything correctly. So I would like to briefly explain what the archive concept looks like:

If a project is archived, then all attached (assigned) variants are read-only. (Here I have just identified a problem, i.e., if a project is archived, there is an auth error if one tries to archive a non-archived variant -> read-only protection applies). However, the variants are not shown as archived.

If a project is not archived, variants can be archived, as the project is not read-only. However, this requires that the user has write permissions.

Archiving means deleted – in other words, only archived variants can be deleted. The same applies to projects. It’s a two-step process, so that on one hand, the user can be really sure that they want to delete something. Or one can define in the process that all work should remain saved, then everything that is no longer needed is archived and that’s it.

@aiAdrian
Copy link
Collaborator Author

Shouldn't archived projects automatically archive variants that aren't archived yet ?
...
The solution would be to automatically archive all the variants of a project that is archived, and tick and disable by default the Show archive button ?

I'm not sure if I understood everything correctly. So I would like to briefly explain what the archive concept looks like:

If a project is archived, then all attached (assigned) variants are read-only. (Here I have just identified a problem, i.e., if a project is archived, there is an auth error if one tries to archive a non-archived variant -> read-only protection applies). However, the variants are not shown as archived.

...

The above reported error fixed is fixed : 4b6c5e5

@@ -171,7 +171,8 @@ describe("TrainrunSection-View", () => {
copyService,
logService,
viewportCullSerivce,
Copy link
Contributor

@SergeCroise SergeCroise Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? (2*7 occurrences of this in the folder editor-main-view)
or is the modification into viewportCullService not the subject of this PR/issue ?

@aiAdrian aiAdrian requested a review from SergeCroise August 27, 2024 15:15
@aiAdrian aiAdrian self-assigned this Aug 27, 2024
@louisgreiner
Copy link
Collaborator

Thanks for your explanations in your previous comment, I better understand the logic : the variants inside an archived project should not be able to be (un)archived afterwards. The user can then see the archived and unarchived variants of an archived project -> working well

…netzgrafik-editor-frontend into 190-bug-archived-read-only-mode-variants-are-editable-even-tho-not-persisted
@aiAdrian aiAdrian merged commit 9472a89 into main Aug 29, 2024
6 checks passed
@aiAdrian aiAdrian deleted the 190-bug-archived-read-only-mode-variants-are-editable-even-tho-not-persisted branch August 29, 2024 12:01
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.

[Bug]: Archived (read-only mode) variants are editable (even tho not persisted)
3 participants