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

Feature/463 multiple users per discipline #1172

Merged
merged 50 commits into from
Dec 15, 2023

Conversation

DimitriDiantos
Copy link

@DimitriDiantos DimitriDiantos commented Sep 26, 2023

To enable the storage of multiple users, follow these steps:

Modify the addButtonSelectionListeners method, createTableColumns method, and the EListStringCellEditingSupport class that extends EStringCellEditingSupport within the UiSnippetRoleManagement class.

add the CustomDialog, which is a custom dialog that extends the Dialog class, allowing user interaction to modify a list of users.

  • The dialog provides options to add, remove, update, and rearrange users in the list. It also notifies the parent application
  • of user changes using a callback mechanism.
  • This dialog is primarily used to interact with and manage a list of features displayed to the user, enabling them to make necessary modifications.
  • This dialog includes the following features:
    • Adding a new feature to the list
    • Removing an existing feature from the list
    • Updating an existing feature in the list
    • Moving a feature up or down in the list
  • The dialog is configured to have a specified size and is centered on the screen for user convenience.
  • Usage:
    1. Create an instance of CustomDialog, providing the parent shell, initial features to display, and a callback for feature updates.
    1. Open the dialog using the open() method.

Correct any errors resulting from the modifications made in the DVLM.Ecore model. Specifically, replace the cardinality of 1 with -1 for the user in the Discipline table.

ngat_di added 4 commits September 15, 2023 14:51
- by modifying the method addButtonSelectionListeners,
createTableColumns and the class EListStringCellEditingSupport which
extends EStringCellEditingSupport from the class
UiSnippetRoleManagement.

-by adding the method setUser inside the Discipline class

- by implementing the method setUser in the class DisciplineImpl
- by modifying the method addButtonSelectionListeners,
createTableColumns and the class EListStringCellEditingSupport which
extends EStringCellEditingSupport from the class
UiSnippetRoleManagement.

-by adding the method setUser inside the Discipline class

- by implementing the method setUser in the class DisciplineImpl
- by updating the RightsHelper class to handle the list of users for a
discipline.
-by setting the list of users accordingly,  when creating a new
discipline in the class UiSnippetRoleManagement
Modify the addButtonSelectionListeners method, createTableColumns
method, and the EListStringCellEditingSupport class that extends
EStringCellEditingSupport within the UiSnippetRoleManagement class.

Correct any errors resulting from the modifications made in the
DVLM.Ecore model. Specifically, replace the cardinality of 1 with -1 for
the user in the Discipline table.
Modify the addButtonSelectionListeners method, createTableColumns
method, and the EListStringCellEditingSupport class that extends
EStringCellEditingSupport within the UiSnippetRoleManagement class.

Correct any errors resulting from the modifications made in the
DVLM.Ecore model. Specifically, replace the cardinality of 1 with -1 for
the user in the Discipline table.
@DimitriDiantos DimitriDiantos linked an issue Sep 26, 2023 that may be closed by this pull request
3 tasks
ngat_di added 5 commits September 26, 2023 17:07
to handle the adding of new user in the Discipline.
Doing it for the clarity of the right syntax to adopt during the add of
a new user in the list of user.
ElistStringCellEditingSupport class, update of the
UiSnippetRoleManagement class and creating of the FeatureUpdateCallback
interface.
discipline

Implement the CustomDialog class, extending the Dialog class to create a
custom dialog that allows user interaction to modify a list of users for
a discipline. The dialog provides functionality to add, remove, update,
move up, and move down users in the list. Additionally, an interface
FeatureUpdateCallback is defined to handle feature update callbacks.

Also, introduce EListStringCellEditingSupport to handle cell editing in
a table viewer for EList of strings in the context of editing a
Discipline.

Files affected:
- CustomDialog.java (new created file)
- FeatureUpdateCallback.java(new created file)
- EListStringCellEditingSupport.java (new created file)
- UiSnippetRoleManagement.java (exited file before the custom Dialog)
- EStringCellEditingSupport (existing before)
@DimitriDiantos
Copy link
Author

Multiple users_dialog

Copy link
Member

@franzTobiasDLR franzTobiasDLR left a comment

Choose a reason for hiding this comment

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

Wow, pretty cool! I really like your changes! Also how you documented your code 👍 I think I found some minor things you could improve... The most importand thing is to make sure you only comited those changes you actually did on purpose... :)

@franzTobiasDLR
Copy link
Member

Nice, now there is the green arrow 👍 🚀

@@ -155,8 +155,8 @@
</eAnnotations>
</eClassifiers>
<eClassifiers xsi:type="ecore:EClass" name="EcoreImport">
<eStructuralFeatures xsi:type="ecore:EAttribute" name="importedNsURI" eType="ecore:EDataType platform:/plugin/org.eclipse.emf.ecore/model/Ecore.ecore#//EString"/>
<eStructuralFeatures xsi:type="ecore:EAttribute" name="importedGenModel" eType="ecore:EDataType platform:/plugin/org.eclipse.emf.ecore/model/Ecore.ecore#//EString"/>
<eStructuralFeatures xsi:type="ecore:EAttribute" name="importedNsURI" eType="ecore:EDataType ../../org.eclipse.emf.ecore/model/Ecore.ecore#//EString"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the change from platform:/plugin to ../.. have some implications? @franzTobiasDLR

Copy link
Member

Choose a reason for hiding this comment

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

No, the difference is just weather you have the dependency in you local worksapce or in the target plattform cache... It will switch automatically... As far as I know we don't have a polixy what to commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I talked with @pchrszon-dlr and we think that the change should be reverted sothat no change shows up in the history

assertTrue("Discipline should be added to Disciplines", repository.getRoleManagement().getDisciplines().contains(discipline));

// Verify that the user is added to the Discipline
assertTrue("User should not be added to Discipline", discipline.getUsers().contains("SampleUser"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the "not" can be removed in the string?

Copy link
Author

Choose a reason for hiding this comment

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

I think the "not" can be removed in the string?

Thanks 👍. I removed the "not".

@dellerDLR
Copy link
Contributor

dellerDLR commented Dec 6, 2023

On small screens not all buttons are visible and the size of the window cannot be adjusted. As a result the Down Button is not visible:
image

Ah nice usability feature would be, if the add button is in focus. Then it would be easier to add multiple users in a short time.

Also I think "users" should be capatilized, since it is a heading.

@DimitriDiantos Despite that it the pull request looks good and can be merged from my side :)

@dellerDLR
Copy link
Contributor

Could you also increase the size of the column? Otherwise only one name is displayed if you dont increase the size manually.
image

@dellerDLR
Copy link
Contributor

I also noticed that the header if you try to add a empty string as a user, displays a wrong error

image

@DimitriDiantos
Copy link
Author

I also noticed that the header if you try to add a empty string as a user, displays a wrong error

image

Thank you for the feedback; I have already corrected it

@DimitriDiantos
Copy link
Author

I also noticed that the header if you try to add a empty string as a user, displays a wrong error

image

Here below is the result:
Screenshot 2023-12-06 173824

@DimitriDiantos
Copy link
Author

Could you also increase the size of the column? Otherwise only one name is displayed if you dont increase the size manually.

Thanks for you comment :). I fixed it. You can find below the result.
Screenshot 2023-12-06 181909
Screenshot 2023-12-06 182007

@DimitriDiantos
Copy link
Author

the size of the window

I
Screenshot 2023-12-06 181909
fixed it.

of the dialog to "USERS_DISCIPLINE" increase the size of the window, so
that all buttons are visible. Add the "add button" as focus.
@dellerDLR
Copy link
Contributor

@DimitriDiantos Looks good. But I think you forgot to push the changes, right?

of the dialog to "USERS_DISCIPLINE" increase the size of the window, so
that all buttons are visible. Add the "add button" as focus.
@dellerDLR
Copy link
Contributor

image

The header of the error dialog has a grammar error.

Could you write the header of the main dialog not on caps (User Discipline)?

Despite that I didn't find any issues.
After that we can finally merge it :)

@DimitriDiantos
Copy link
Author

image

The header of the error dialog has a grammar error.

Could you write the header of the main dialog not on caps (User Discipline)?

Despite that I didn't find any issues. After that we can finally merge it :)

Thanks for your comment. I already changed it. you can find below the result. :)
changes

@dellerDLR
Copy link
Contributor

Hey @DimitriDiantos ,
I just noticed that it is possible to edit the discipline names, even if I do not have the proper permissions.
I think there is a check missing before opening the dialog, if the user has the right to change the role management.

It is not saved on the disk, but may cause confusion.

image

As you can see, I can open the dialog, even though I do not have the permission to press the other buttons.

@dellerDLR
Copy link
Contributor

I even managed to make me the system engineer, which gives me the right to change the system model

image

image

While doing this, the changes even get saved to the disk.

@dellerDLR
Copy link
Contributor

You probably ahve to check the EditingSupport of the table.
Maybe tou forgot to use the RoleManagementCheck Command

@DimitriDiantos
Copy link
Author

Hey @DimitriDiantos , I just noticed that it is possible to edit the discipline names, even if I do not have the proper permissions. I think there is a check missing before opening the dialog, if the user has the right to change the role management.

It is not saved on the disk, but may cause confusion.

image

As you can see, I can open the dialog, even though I do not have the permission to press the other buttons.

I fixed by adding a method to check, if the actual user belong to the list of users. else he cannot make a change. below you can find the screen of the result.
change1
change2

@DimitriDiantos
Copy link
Author

Hey @DimitriDiantos , I just noticed that it is possible to edit the discipline names, even if I do not have the proper permissions. I think there is a check missing before opening the dialog, if the user has the right to change the role management.
It is not saved on the disk, but may cause confusion.
image
As you can see, I can open the dialog, even though I do not have the permission to press the other buttons.

I fixed by adding a method to check, if the actual user belong to the list of users. else he cannot make a change. below you can find the screen of the result. change1 change2 i also solved the problem for the refresh button. you can check it. :).

access can make modifications,and solving the refresh problem.
@DimitriDiantos
Copy link
Author

Hi @franzTobiasDLR @dellerDLR Now the data is stored in the database and when you press the refresh button it always displays all the data.

@dellerDLR dellerDLR merged commit 8027909 into development Dec 15, 2023
14 checks passed
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.

Multiple users per discipline
3 participants