Review Commissions page renders no text

[japacible]

Go to commissions/review/{number} from the commissions dashboard.

Observe that there is no text that is rendered. It's just a blank page after the header.

wip-commissions branch

[jbrodhacker]

7067a09 fixes this. All the containers were inside of the "if current_user.id == @artist.id" without adding a section for the else case (previously, select containers were placed outside that if statement). I encapsulated the else branch containers within a form, but this was done stylistically rather than out of necessity (i.e. front end devs: feel free to change it).

[Bejoty]

Instead of a blank form could we just have an error message noting that the pager is unviewable as that user or something?

[jbrodhacker]

It's not a blank form; it's a form that contains the choices and the final step, but does not include the containers for the price change, revision change, or the accept/revise/decline buttons.

[Bejoty]

Oh, I guess I'm not getting when this would be viewed. Isn't the else in case that URL is accessed by someone who isn't the artist?

[jbrodhacker]

The controller already performs that check, this is when a commissioner is trying to look at the commission that they created. (The artist view of the review page was just fine).

[jbrodhacker]

Er, what I just said is not completely true; the controller checks if the user isn't logged in. The change I made caused it such that anyone who is not the artist can simply view the commission. If we want to make it private, however, (a.k.a. between commissioner and artist only) I can add that logic into the controller.

[jbrodhacker]

And for that case I would redirect the user with an alert that they cannot view that page or some such thing.

[Bejoty]

Ah, okay. So from the customer's perspective, before the review happens, they go to the review page to view their request?

I'm just wondering if it would be easier or trivial to put that functionality on what will end up being the revision page. For instance, the "Sent Commission Requests" links on the dashboard could go to some ../commissions/requests/[#] -- being the customer view that adds a revision function when the object status reaches that point -- instead of the same ../commissions/review/[#] that the artist sees.

The alternative I assume is the customer seeing a link on the review page that brings them to the revision page, though at that point we could just make the same page do everything.

Thoughts?

[Bejoty]

In fact, I don't think it would be that unreasonable to put the review, revision, and progress functionality on the same page, and just vary what view appears based on the user ID and commission object status.

Is that insane? Or maybe that's the most logical?

[jbrodhacker]

It's certainly doable, at least for review & revision. Since I don't know what progress is going to look like, we may want to make a different page for it that can utilize the same javascript partial the review uses to generate all the previous information.

[jbrodhacker]

I'm not particularly sure what's easiest at this point.

[Bejoty]

Yeah, assuming the more advanced progress page is a stretch goal, the eventual expansion of the view would make this all-in-one idea pretty bulky.

But as far as the revision/review process goes, they're almost doing the same thing already, just adding strings to different arrays within the object. I say we go ahead and make them in the same view.

[Bejoty]

The HTML could do something like this:

if current user == user who can view this request (two parties or public)
  display commission request

  if current user == artist && commission status == review
    display review input form

  if current user == customer && commission status == revision
    display revision input form

And then I'll just need to add the current list of artist's previous reviews, which should probably be viewable to both parties. Probably should have already been there actually. And I'll need to cover the edge case of the customer view before the artist has given any reviews.