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

added auction list to admin panel #2341

Merged
merged 11 commits into from
Apr 28, 2022
Merged

added auction list to admin panel #2341

merged 11 commits into from
Apr 28, 2022

Conversation

OlegPhenomenon
Copy link
Contributor

@OlegPhenomenon OlegPhenomenon commented Apr 8, 2022

Not for merge yet.
Need to add tests

@viezly
Copy link

viezly bot commented Apr 8, 2022

This pull request is split into 6 parts for easier review.
👀 Review pull request on Viezly

Changed files are located in these folders:

  • app
  • app/controllers
  • app/models
  • app/services
  • config
  • db
  • test/integration/api/v1/auctions

@OlegPhenomenon OlegPhenomenon force-pushed the auction-page-to-admin branch 4 times, most recently from 2c8d8b6 to 83c04cd Compare April 13, 2022 08:39
@vohmar
Copy link
Contributor

vohmar commented Apr 20, 2022

looking good. Few thought and issues:

  • please move the auctions selection in the settings menu down to system section between zones and blocked domains
  • the default view shows only auctions with no_bids even though status selection is empty (maybe because the whole list is not returned and the list starts with records with no_bids status)
  • pagination is missing
  • Reset btn rename to reset or reset filter; button tooltips missing ("translation missing")
  • csv upload is not working - internal error. Maybe because of invalid csv format (some tip or hint should be there somewhere to inform user on what the proper csv import file looks like)
  • created_ at from and created at until and results per page options do not work

@vohmar
Copy link
Contributor

vohmar commented Apr 26, 2022

looking even better

  • registration code tends to extend the column so that other like create date are wrapped into more lines making the whole table look bad - maybe its better to fix the width of reg code field and provide some copy help for the content

image

image

  • Type automatically should be replaced with automatic or auto; manually with manual
  • trying to upload csv for adding new auctions - getting "invalid csv format" error. What is correct format? We should add some help about this in the UI
  • when adding an auction from reserved list to auction there should be a warning to let user know that it is reserved and as a result this domain will be removed from the reserved list
  • blocked domains should not be possible to add to the auction
  • registered domains should not be possible to add to the auction
  • domains in active disputed list should not be possible to add to the auction
  • only domains under the zones served by the registry should be possible to add to the auction and only the first level ie domeen.ee or domeen.pri.ee - was able to add domains like aaaa.e, aaaa, aaa.aaa.aaa
  • is it possible to refresh the list automatically when new domain(s) is/are added to the list ie my list was sorted by created at date descending, but adding newly added domains did not automatically appear on top of the list , had to reorder the list to see if and what was added.

--

import from reserved list is crazy good, buttons work csv download works - nicely done!

@OlegPhenomenon OlegPhenomenon force-pushed the auction-page-to-admin branch 2 times, most recently from 8c697cb to b15e8a6 Compare April 27, 2022 09:00
@vohmar
Copy link
Contributor

vohmar commented Apr 27, 2022

Getting close to amazing. Few things still though:

  • sorting by type does not work - auto always at the top, manual at the end
  • still missing warnings when adding domains from reserved list to auction

@OlegPhenomenon OlegPhenomenon marked this pull request as ready for review April 28, 2022 07:33
@vohmar vohmar merged commit 4fe22ec into master Apr 28, 2022
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.

2 participants