License Management is showing the list of new licenses in the MR widget (https://gitlab.com/gitlab-org/gitlab-ee/issues/5487), and we can see if we are introducing any new license because of some new dependency.
Users should be able also to define which licenses are approved and which ones are blacklisted.
Proposal
Define a way to approve or blacklist licenses. This should be stored in the project settings, and potentially be leveraged by future iterations to get trends and provide suggestions to other projects as well (Signal to Noise).
When showing results in the MR widget, items can be of three kinds:
approved (green): the license for this item has already been approved
blacklisted (red): the license for this item has already been blacklisted
unapproved (grey/black?): the system doesn't have information for this license, some action should be taken
Design
MR widget – License management
Approve license dialog
Clicking on any of the licenses in the MR widget opens the approve license dialog.
Project Settings (collapsed)
Project Settings (expanded)
Remove license modal
Project Settings (no licenses)
Latest Sketch file can be found here and design specs here.
This page may contain information related to upcoming products, features and functionality.
It is important to note that the information presented is for informational purposes only, so please do not rely on the information for purchasing or planning purposes.
Just like with all projects, the items mentioned on the page are subject to change or delay, and the development, release, and timing of any products, features, or functionality remain at the sole discretion of GitLab Inc.
@plafoucriere what do you think is a good approach for approvals/blacklist?
A file in the repo allows you to bring it with your project, so cloning etc. will keep this information. But having it in the UI allows you to easily add/remove items.
I'm also wondering if we want to show approved new items: it is valuable to know that we have new dependencies that are passing the license check, but we probably want to focus more on possible problems. Hiding new approved items could be an option to keep the report smaller.
We need to decide on gitlab-org/gitlab-ee#5151 first, as we will reuse the same pattern. I'm seeing this in a config file, the one we should adopt in the whitelisting issue.
Showing approved (whitelisted) new items is the same question for any other report that will be using whitelisting. We should do something very generic that will be the same for all these reports. Also, this will improve the UX, as the user will have always the same way to deal with issues, whether they be for security or licenses.
And like security alerts whitelisting, a change in the approvals/blacklists should be reviewed and approved. So a file with Merge Requests seems to be the solution for me.
@plafoucriere I'm not sure this should be considered at the same way of security features.
For security features, you dismiss an item probably because it is a "false positive". In this case, you approve or not a license because it fits the specific project licensing requirements.
I still can see some chances to get useful information out of user choices: if you project has a specific license, we can automatically suggest you a "scoring" for other licenses based on what users did for similar projects.
@plafoucriere I saw you mentioned #5151 (closed), was this already resolved or do we don't need that anymore? From a timeline perspective, what parts do we have already as an API and/or can we start directly with the implementation when UX is available?
@timzallmann It's going to be new api endpoints for this one. #5151 (closed) was only about vulnerabilities, here we're working with licenses instead. We don't have any api ready, as we're starting this at the same time as your team.
You shouldn't wait for our implementation to be ready, there are probably some subs you can use to get started. Otherwise, we'll create a strong dependence between our two teams, and end up in a complete waterfall process, which is what we're trying to avoid if I understand correctly.
@plafoucriere Totally agree that we can stub a lot when we are able to define it nicely at the beginning in a kickoff meeting. We try to get a feeling for the timeline so that the connecting doesn't happen at the total end of the release cycle and/or if we maybe even need to split it into 2 cycles.
This issue is the only one we are doing the "old" way (vs the new process we put in place last week). That means we're waiting for UX to be "Ready" before starting to work on it. Maybe @jkarthik can share an estimate?
We're shutting down Gemnasium next week, so the SP team will be pretty busy working on a different roadmap, do not expect something from us before 2-3 weeks :(
Thx for the info, then I would rather suggest that this is a Stretch item at least for frontend, wdyt @bikebilly? Because when the SP team has a different priority it is even harder to do a kickoff decide on API design, etc.
Thx for the info, then I would rather suggest that this is a Stretch item at least for frontend, wdyt @bikebilly? Because when the SP team has a different priority it is even harder to do a kickoff decide on API design, etc.
If this gets marked as a Stretch@plafoucriere, it will be a stretch for UX as well. That means it will not get touched until everything else has been completed.
@timzallmann@plafoucriere@sarrahvesselov I understand that the work on this issue has different stakeholders and it should sync as soon as possible to give the proper time to each team, but I'm considering this a very important iteration for our Product Vision 2018.
This is also our main direction item for %11.0, and I'd really love to see it done in this release since otherwise the license management feature shipped in %10.8 cannot reach an MVP state.
What do you think? @plafoucriere could you please consider to provide just the information needed by @timzallmann so the frontend team is not blocked? We are still requiring some UX, so maybe we could use these days to define the engineering part of the story.
I'm wondering if we can try to find a different word instead of "unapproved". I feel it can be confused with something that you specifically didn't want (alias for blacklisted), while it is just something you haven't defined yet.
Maybe we can also keep it simpler and don't give the option to mark as "unapproved" again. Once you take a decision, you can switch between "approved" and "blacklisted" in the modal popup. In settings, you can switch between "approved" and "blacklisted", or delete the entry (instead of marking as "unapproved". This approach can also limit the usage of the "unapproved" word, solving my previous concern.
Recap of my suggested changes:
in the modal form, remove the option to mark as "unapproved"
in the settings page, remove the option to mark as "unapproved"
in the settings page, add the option to delete an entry
We could also consider adding an entry manually in the settings page, but let's decouple in another issue for a future iteration.
Thanks for the feedback @bikebilly. It makes sense to have two statuses (approved/blacklisted) for the user to choose from. I'll update the design accordingly.
We could also consider adding an entry manually in the settings page, but let's decouple in another issue for a future iteration.
We'll need to have a unique id for a license to help prevent duplicates. The license URL may be the best bet here. Let's discuss this in a future issue.
@bikebilly I've updated the designs and removed the option to mark a license as "unapproved". I've added a confirmation dialog when removing a license and a screen for when there are no approved/blacklisted licenses in the project.
I see it could maybe useful to add a blue question mark to the settings section and in the approval popup, pointing to documentation. What do you think?
Have you thought about the order in which licenses are displayed depending on status?
Is the status header necessary in the table in settings? It might be possible to shove the actions towards the delete action... or otherwise make the buttons all the same width
I believe @cperessini had a similar design challenge with the secret variables table
I see it could maybe useful to add a blue question mark to the settings section and in the approval popup, pointing to documentation.
@bikebilly Makes sense. I'll add them to both settings and the approval popup.
Have you thought about the order in which licenses are displayed depending on status?
The licenses are ordered alphabetical. This is the best option as similar licenses (like BSD) will appear closer to each other and help the user scan through the list quickly.
Is the status header necessary in the table in settings?
@dimitrieh I added this to bring visual balance the table header. I did not have this in the earlier iteration when there was an extra column.
It might be possible to shove the actions towards the delete action... or otherwise make the buttons all the same width
@dimitrieh I'll try this. It will definitely make the dropdowns look better. The reason I made the dropdowns different widths was that I couldn't find any guidelines in the UX Guide for button widths, especially inside a table.
@bikebilly I've updated the project settings pages with the help icon. @dimitrieh I've made the buttons equal width and changed the delete button to fit our convention.
Approve license modal
I've added the help link as a text instead of an icon button (as used in other places). If we were to use the icon button style, the best placement for it would be the title of the modal which, in this case, ends in a question mark. Rather than have two question marks next to each other, I placed this as a pretty descriptive text link in the body of the modal.
@mikegreiling please take care of this issue, its the continuation of #5487 (closed) that @kushalpandya is working on right now. He is most probably during next week so please connect and take it from there.
Please do a kickoff meeting with @plafoucriere or whom will be assigned from SP to create a API mock if needed.
I believe @cperessini had a similar design challenge with the secret variables table
@dimitrieh I didn't work on the original design for the secret variables table, so you might be thinking of someone else. I did encounter a similar situation with the list of clusters from https://gitlab.com/gitlab-org/gitlab-ee/issues/3734. I didn't include a Status header in the end, but like @jkarthik mentioned, there are more columns in that table, so it's a bit of a different situation.
Just to give my opinion, I don't feel this table needs the Status header to provide visual balance. Since the right-hand side of the table is detached from the left and only contains buttons, I think it works well without a header. I think you can go either way
Just to give my opinion, I don't feel this table needs the Status header to provide visual balance. Since the right-hand side of the table is detached from the left and only contains buttons, I think it works well without a header.
I tend to agree here @jkarthik though I don't have a strong opinion. Please also consider that this will be the ONLY place in the app that contains a header like this. That will cause inconsistency that we will need to address.
@bikebilly (/cc @timzallmann), we just finished the full planning for 11.0 (I know, we're late).
We don't have the capacity for such a bug feature with the current size of the team.
I will share the Utilization we have evaluated for this month, and this one simply can't fit.
I suggest postponing to 11.1, please.
In the meantime, I'm setting Stretch for this one. Sorry for the late notification, we're trying to anticipate more, and we won't be able to if we're always one step behind.
The UI is similar in flow to #5151 (closed). The approve license dialog opens when the user clicks on the license name in the MR widget. Additionally, there is some new functionality in the project settings page, under CI/CD. This is to approve/blacklist licenses.
@timzallmann Frontend is probably really similar but current code is also really tied to the security reports. I think it would be better handled by a frontend member and on my side I need to focus on another feature.
Isn't UX here already done with the same as #5151 (closed) (closed)?
I don't think this has been already addressed before. There are no reference to License Management in that issue. This is not a security report, and it differs in functionalities and design. It also includes specific settings pages that should be implemented as new.
It probably has similarities with the security features, and maybe some frontend components can be reused. Anyway, we may expect in the future that license management and security reports will diverge significantly.
@mikegreiling@groulot Do you have everything you need/is everything clear? Asking as there still is some confusion in the latest few comments (which seems to be addressed but not totally sure )
@leipert can you please make sure you're working in this branch too?
FYI, we finish development before the end of each month, so we give enough time to reviewers and maintainers before code freeze. Could you tell me if we're still on schedule to meet that target? Thanks
@plafoucriere I (almost) finished the other deliverable. Will give that into review today, and start working on this. I assume that it should be done rather fast as well!
@jkarthik Looking at the mock ups I have the following questions:
Reference
Question
I am a bit unfamiliar, do we use question marks in modals
Is it necessary to repeat the license name in the modal header? License names can be really long, especially if not normalized and I feel this could break the header
Is it wanted that the delete button has no background?
I am a bit unfamiliar, do we use question marks in modals
Yes, we do. Here is a link to the deprecated UX guide. Modals haven't been defined yet in the new design system. So its best be remain consistent.
Is it necessary to repeat the license name in the modal header? License names can be really long, especially if not normalized and I feel this could break the header
Thanks for catching this. Let's use the title Approve license? and Remove license? for the respective modal headers and avoid using full license names in the title. I'll update the issue description/mockups shortly.
Is it wanted that the delete button has no background?
Again, this is for the sake of consistency. The delete button without border is used in many places in GitLab. These will have a larger hit area similar to the icons in the main navbar, top-right corner. The icon is 16x16px and the hit area is 32x32px. You can refer to the specs as well.
@jkarthik Thanks for the answers, makes sense . The "trash icon" button still feels funky, as I have never stumbled across those, I always red buttons with "Remove" or "Delete". Personally I don't like buttons with only icons. What places are you referring to? The rather new badges UI uses a red background, and other pages as for destructive actions:
@jkarthik sorry about the delay. I don't think this is documented anywhere yet, but I have also seen both used.
I know at some point we discussed using secondary buttons to avoid the visual overload that red primary buttons can cause, but I can't remember seeing that in the app, so maybe the proposal was abandoned. @hazelyang I think you were part of that discussion, do you know if any conclusions were reached?
I would agree that icon-only buttons are less desirable because they're not as intuitive, but I think they work well in this case and we can use them in instances like this until we decide otherwise.
Thanks, @cperessini@hazelyang. I'll find some time and create a proposal for this soon. In the meantime, let us stick with the icon-only buttons @leipert.
RSpec tests are working. There is a managed_license controller, and also an API endpoint (If you end up only using the API, tell me so I remove the controller)