WIP: Improve management of artifacts

What does this MR do?

Original https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18707

Related https://gitlab.com/gitlab-org/gitlab-ce/issues/48862

Screenshots

Does this MR meet the acceptance criteria?

Conformity

• Changelog entry for user-facing changes, or community contribution. Check the link for other scenarios.
• Documentation created/updated or follow-up review issue created
• Code review guidelines
• Merge request performance guidelines
• Style guides
• Database guides
• Separation of EE specific content

Performance and Testing

• Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
• Tested in all supported browsers

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

• Label as security and @ mention @gitlab-com/gl-security/appsec
• The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
• Security reports checked/validated by a reviewer from the AppSec team

changed milestone to %12.3

changed the description

added CI artifacts Community contribution devopsverify grouppipeline execution typefeature labels

	4 Warnings
⚠	This merge request does not have any assignee yet. Setting an assignee clarifies who needs to take action on the merge request at any given time.
⚠	This merge request is quite big (more than 787 lines changed), please consider splitting it into multiple merge requests.
⚠	CHANGELOG missing. You can create one with: bin/changelog -m 32131 "Improve management of artifacts" If your merge request doesn’t warrant a CHANGELOG entry, consider adding any of the ~backstage, ci-build, ~Documentation, meta, QA, test labels. See the documentation.
⚠	This merge request changed files with disabled eslint rules. Please consider fixing them.

Disabled eslint rules

The following files have disabled eslint rules. Please consider fixing them:

• app/assets/javascripts/pages/constants.js

Run the following command for more details

node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \
  'app/assets/javascripts/pages/constants.js'

Reviewer roulette

Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.

To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.

Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.

Category	Reviewer	Maintainer
frontend	Scott Hampton (@shampton)	Kushal Pandya (@kushalpandya)
backend	Felipe Artur (@felipe_artur)	Sean McGivern (@smcgivern)
test for spec/features/*	No reviewer available	No maintainer available

Generated by 🚫 Danger

added 6 commits

• 60955ccb - Implement artifacts page
• 29f7e9af - Fix rebase mistakes
• a09a4fe4 - Resolve spec failures
• 5d6d64c7 - Update locales file
• 785ae2f5 - Fix lint errors
• b3e29bb3 - Add missing frozen string literals

Compare with previous version

mentioned in merge request !18707 (closed)

Current state

Just had a talk with @matteeyah on this merge request and it seems there is some disparity between what the original description screenshot of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18707/https://gitlab.com/gitlab-org/gitlab-ce/issues/48862 showed and what @matteeyah and @shampton were able (up until now) to recover from it into this new merge request:

Original	Current state

What is needed?

The original issue focusses in on the "Ability to query and delete artifacts directly from UI". Plural is key here, as the main use case seems to revolve around mass deleting unuseful artifacts.

Batch select and per-column-filtering are deemed essential for that. Aside from that, there are quite some usability and visual improvements to be made.

Note, that this talks about what is needed for a true MVC, not what can be delivered in the first iteration. Potentially we might need 1-2 milestones to get there.

What are the next steps?

@matteeyah is going to try to see if they can recover more from the originally showed functionality.

I will look into a potential design for this, as the column filtering paradigm is currently non-existent within GitLab. As mentioned by @matteeyah it might be a plausible idea to use filter tokens in order to be able to narrow down search per column.

cc: @shampton @vkarnes @jlenny

I have looked at the design and have considered what is necessary. Imo we do not need to use any new paradigms and can completely rely on existing search paradigms and components for this feature.

However, similar to https://gitlab.com/gitlab-org/gitlab-ce/issues/18054 this might make us reliant on the effort of gitlab-org/gitlab-services/design.gitlab.com#272 (closed) which seems it will be delivered in 12.3.

I have expanded a bit on what is necessary, taking inspiration from https://gitlab.com/gitlab-org/gitlab-ce/issues/18054. The intent here is to converge on what is shown and see what is feasible to deliver in the near future.

Search:

• Job name

Filter:

• Branch
• Created date
• Commit author
• Expired date
• Pipeline ID
• Size
• Tag
• Trigger author
• Trigger source

Sort:

• Branch
• Created date
• Commit author
• Expired date
• Pipeline ID
• Size
• Tag
• Trigger author
• Trigger source

Other functionality:

• Batch select, including delete all button
• Recent searches
• Total artifact size depending on filtering

Shown data per artifact row:

• Job ID
• Job Name
• Triggerer (also called trigger source, can include API, web, push, etc)
• Pipeline ID
• Pipeline ref
• Commit
• Commit author
• Commit title
• Size
• Created time stamp
• Expired time stamp
• Download artifact button
• Browse artifact button
• Delete artifact button

Functionality which is not shown in mockups:

• Total count of selection
• Pagination, including how this works with batch selecting all currently filtered items across potentially multiple pages (if any).

---

@matteeyah @shampton I would love to get some feedback on the shown capabilities in the mockups. Original functionality which was highlighted in the original screenshot is in bold. Again let's see what we can scope down to get to a reasonable scope which is deliverable.

A good step, in my opinion, would be to see if we can show all the data displayed in the mockup (including making it look like the mockup), then focus in on just 1-2 filter options, including sorting and batch select with delete.

If need be let's schedule a meeting so we can speed this discussion up

cc: @vkarnes @jlenny

@dimitrieh - @matteeyah brought up a good point in slack. Maybe for the first iteration, we should focus on refactoring the current functionality to get it to a passing state and then add more capabilities in followup issues. Does that sound alright with you?

That being said, I think the mockups you made look great.

thanks @shampton. Would you be able to elaborate on the feasibility of features shown in the mockups? I listed them out as well. In order to see what we can do when and in how many iterations I would need technical input.

The same would go for @matteeyah from a BE perspective.

Maybe for the first iteration, we should focus on refactoring the current functionality to get it to a passing state and then add more capabilities in followup issues. Does that sound alright with you?

Imo we can merge, with the current functionality if cleaned up well and working consistently. We can, however, not close the issue as the new view is not solving the problem. I would be more at ease with merging it behind a feature flag so we can bring it to the MVC level at which point it becomes viable.

I do understand the want to merge this in order to iterate on it separately. Let's include @jlenny in here as well.

From my perspective, after merging the initial iteration, there's two more iteration we need to follow-up with to get the functionality described in the latest mockup:

1. Filtering and sorting
2. Batch actions

---

I opened an issue to track the effort ongoing in this iteration - which is just getting the artifacts page https://gitlab.com/gitlab-org/gitlab-ce/issues/66613

@dimitrieh I agree with @matteeyah's comment above. Those two iterations sound good to me.

My preference would be to change the order, solve the problem in the planned issue first and then later redesign it to look/work nicer.

There might have been some miscommunication - we're not really trying to redesign this functionally. By "refactor" Scott and I meant to get the code for the current functionality into an acceptable/mergeable state. This is required prior to adding the missing functionality.

In addition to this, there's really no way we could both refactor this and add the missing functionality in a single iteration (this is my opinion WRT to the backend), which is why I proposed we focused on refactoring now, then following up with adding the missing bits.

@jlenny @dimitrieh @matteeyah It seems like there's a lot of back and forth on this for what the proper direction is for this milestone. Should we consider doing a quick sync meeting to discuss more in depth so we can @matteeyah and I can start working? It's getting late in the milestone, and I want to make sure this is prioritized properly.

If we aren't able to finish this issue in this milestone, we should try to get as much done as possible and then finish it in %12.4. From an engineering perspective I trust you all to come up with the right implementation plan so don't need to second guess that.

If there's a way to reduce scope in a way to where we can achieve the stated purpose of the change in %12.3 that would be my preference but it sounds like that's not possible.

@shampton Whatever we decide to do in the current milestone I think the first step is going to be to refactor the code that we currently have and get that merged first. We can discuss the end goal for this Deliverable for the current milestone parallel to starting work on refactoring this chunk of code.

@shampton @matteeyah Afaik we are now aligned to the following:

For 12.3:

1. Make current code production-ready. This includes

• Basic artifact list view
• A search bar with working "past searches" and one "filtering" token type
• Basic sort on created date
• Pagination?
• Implemented with feature flag turned off

Stretch 12.3 and potential follow up:

1. Batch actions
2. Filtering and sorting
3. Update table with information and styling (this will need to be updated depending on the filters we implement)

I switched around batch actions and filtering/sorting in priority as the batch selecting is key to make any filtering work. In that same mind, I would say that batch actions are the most welcome addition for 12.3.

I'll move the concept from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/32131#note_207391838 towards the issue.

cc: @jlenny

mentioned in issue #48862 (moved)

mentioned in issue #66613 (moved)

mentioned in commit 17d8ebd4

closed

I closed this MR since we'll proceed with separating the backend and frontend portions of this code and working on them separately.

The backend MR is in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/32590

mentioned in commit fe9fef55

mentioned in merge request !32666 (closed)